Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot properly escape PRAGMA command parameters #579

Closed
lasselindqvist opened this issue Jan 16, 2021 · 5 comments
Closed

Cannot properly escape PRAGMA command parameters #579

lasselindqvist opened this issue Jan 16, 2021 · 5 comments
Labels
waiting for feedback Waiting for a feedback from the issue creator

Comments

@lasselindqvist
Copy link

SQLite has SQL extension called PRAGMA. It mostly takes enum and boolean parameters and these cases do not need prepared statements and arbitrary parameter escaping.

But some pragmas take string parameters. One example https://www.sqlite.org/pragma.html#pragma_temp_store_directory
This one is deprecated and maybe not relevant to be handled properly anymore.

But when using encrypted databases the encryption key is set with
PRAGMA key='your-secret-key';
Now, if this key contains ' characters the command fails. See test testSetPragmaNotEscaped. Escaping the key input with the method from org.sqlite.core.CoreDatabaseMetaData#escape makes it work. The method is just not available. I guess to not encourage people to use that but use prepared statements properly. See testSetPragmaEscaped.

As testSetPragmaParameter shows, using prepared statements to escape the contents is not supported. We get

org.sqlite.SQLiteException: [SQLITE_ERROR] SQL error or missing database (near "?": syntax error)
	at org.sqlite.core.DB.newSQLException(DB.java:1011)
	at org.sqlite.core.DB.newSQLException(DB.java:1024)
	at org.sqlite.core.DB.throwex(DB.java:989)
	at org.sqlite.core.NativeDB.prepare_utf8(Native Method)
	at org.sqlite.core.NativeDB.prepare(NativeDB.java:134)
	at org.sqlite.core.DB.prepare(DB.java:257)
	at org.sqlite.core.CorePreparedStatement.<init>(CorePreparedStatement.java:45)

So my question is:

  1. Should using prepared statements for Pragma parameters be supported?
  2. If so, should the fix be in SQLite itself, not here?
  3. If not, should there be a way to have some helper method to use for escaping in these kind of situations, or would it encourage bypassing prepared statements when they should be used?
package org.sqlite;

import java.io.File;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.Statement;
import java.util.Properties;

import junit.framework.Assert;
import org.junit.BeforeClass;
import org.junit.Test;

public class PragmaTest
{
	
    @Test
    public void testSetPragmaParameter() throws Exception {

        // create a memory database
        File tmpFile = File.createTempFile("test", ".sqlite");
        tmpFile.deleteOnExit();
    	
    	Connection conn = null;
    	try {
    		conn = DriverManager.getConnection("jdbc:sqlite:" + tmpFile.getAbsolutePath());
    		String sql = "pragma defer_foreign_keys = ?";
    		try (PreparedStatement ps = conn.prepareStatement(sql);) {
    			ps.setBoolean(1, true);
    			ps.execute();
    		}

	    }
	    finally {
	        if (conn != null)
	            conn.close();
	    }
    }
    
    @Test
    public void testSetPragmaDirect() throws Exception {

        // create a memory database
        File tmpFile = File.createTempFile("test", ".sqlite");
        tmpFile.deleteOnExit();
    	
    	Connection conn = null;
    	try {
    		conn = DriverManager.getConnection("jdbc:sqlite:" + tmpFile.getAbsolutePath());
    		String sql = "pragma defer_foreign_keys = true";
    		try (PreparedStatement ps = conn.prepareStatement(sql);) {
    			ps.execute();
    		}

	    }
	    finally {
	        if (conn != null)
	            conn.close();
	    }
    }

    @Test
    public void testSetPragmaEscaped() throws Exception {

        // create a memory database
        File tmpFile = File.createTempFile("test", ".sqlite");
        tmpFile.deleteOnExit();
    	
    	Connection conn = null;
    	try {
    		conn = DriverManager.getConnection("jdbc:sqlite:" + tmpFile.getAbsolutePath());
    		String param = escape("my'pass''word");
    		String sql = "pragma key = '" + param + "';";
    		try (PreparedStatement ps = conn.prepareStatement(sql);) {
    			ps.execute();
    		}

	    }
	    finally {
	        if (conn != null)
	            conn.close();
	    }
    }
    
    @Test
    public void testSetPragmaNotEscaped() throws Exception {

        // create a memory database
        File tmpFile = File.createTempFile("test", ".sqlite");
        tmpFile.deleteOnExit();
    	
    	Connection conn = null;
    	try {
    		conn = DriverManager.getConnection("jdbc:sqlite:" + tmpFile.getAbsolutePath());
    		String param = "my'pass''word";
    		String sql = "pragma key = '" + param + "';";
    		try (PreparedStatement ps = conn.prepareStatement(sql);) {
    			ps.execute();
    		}

	    }
	    finally {
	        if (conn != null)
	            conn.close();
	    }
    }
    
    /**
     * This method is from {@link org.sqlite.core.CoreDatabaseMetaData#escape}.
     * 
     * The method is not public and cannot be thus used elsewhere.
     **/
    private String escape(final String val) {
        // TODO: this function is ugly, pass this work off to SQLite, then we
        //       don't have to worry about Unicode 4, other characters needing
        //       escaping, etc.
        int len = val.length();
        StringBuilder buf = new StringBuilder(len);
        for (int i = 0; i < len; i++) {
            if (val.charAt(i) == '\'') {
                buf.append('\'');
            }
            buf.append(val.charAt(i));
        }
        return buf.toString();
    }

}

@gotson
Copy link
Collaborator

gotson commented Jul 27, 2022

There is already support for a synthetic pragma called PASSWORD, see

stat.execute(String.format(passwordPragma, password.replace("'", "''")));
and
PASSWORD("password", null);

@gotson gotson added the waiting for feedback Waiting for a feedback from the issue creator label Jul 27, 2022
@lasselindqvist
Copy link
Author

The summary here is that SQLite supports pragmas, pragmas can have parameters for them. If the parameter is boolean, you can give it is a parameter and bind it properly, but if it is string, you cannot give it as a parameter, instead you need to give the full SQL directly, meaning there is the possibility to forget to escape the parameter properly.

Maybe doubling quotes is enough to escape, I am not 100 % sure about that, but do not have any counter examples ready. Also I am not sure if the problem is even solvable here, maybe it would need to be solved in SQLite directly.

@lasselindqvist
Copy link
Author

And why not just use the pragma when connecting? For my special case it happened when trying to "rekey" a database, so it was not possible to do it via the config options.

@gotson
Copy link
Collaborator

gotson commented Jul 28, 2022

Also I am not sure if the problem is even solvable here, maybe it would need to be solved in SQLite directly.

That might be the first thing to check on your side, ie confirm if it's a SQLite problem, or this driver's problem.

And why not just use the pragma when connecting? For my special case it happened when trying to "rekey" a database, so it was not possible to do it via the config options.

Not sure to understand this.

Maybe doubling quotes is enough to escape, I am not 100 % sure about that, but do not have any counter examples ready.

If you can also give it a try, that would be good.


I will leave this as 'waiting for feedback' until you can provide some more details on the points above.

@gotson
Copy link
Collaborator

gotson commented Jan 13, 2023

Closing this as no feedback was received.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for feedback Waiting for a feedback from the issue creator
Projects
None yet
Development

No branches or pull requests

2 participants