Merge pull request #95 from abeluck/sql_injection

Fix potential SQL injection when keying the database

Author: developernotes

jni/Android.mk

@@ -16,6 +16,9 @@ endif
 
 include $(CLEAR_VARS)
 
+# expose the sqlcipher C API
+LOCAL_CFLAGS += -DSQLITE_HAS_CODEC
+
 LOCAL_SRC_FILES:= \
 	net_sqlcipher_database_SQLiteCompiledSql.cpp \
 	net_sqlcipher_database_SQLiteDatabase.cpp \
@@ -37,6 +40,7 @@ LOCAL_C_INCLUDES += \
 	$(EXTERNAL_PATH)/platform-system-core/include \
 	$(LOCAL_PATH)/include \
 	$(EXTERNAL_PATH)/platform-frameworks-base/include \
+	$(EXTERNAL_PATH)/icu4c/common \
 
 LOCAL_SHARED_LIBRARIES := \
 	libcrypto \
@@ -53,8 +57,9 @@ LOCAL_LDLIBS += -ldl -llog
 LOCAL_LDLIBS += -lnativehelper -landroid_runtime -lutils -lbinder
 # these are build in the ../external section
 
-#LOCAL_REQUIRED_MODULES += libsqlcipher libicuuc libicui18n
-LOCAL_LDLIBS += -lsqlcipher_android
+LOCAL_LDLIBS  += -lsqlcipher_android
+LOCAL_LDFLAGS += -L../obj/local/$(TARGET_ARCH_ABI)
+LOCAL_LDLIBS  += -licui18n -licuuc
 
 ifeq ($(WITH_MALLOC_LEAK_CHECK),true)
 	LOCAL_CFLAGS += -DMALLOC_LEAK_CHECK

jni/net_sqlcipher_database_SQLiteDatabase.cpp

@@ -37,6 +37,10 @@
 #include <string.h>
 #include <sys/ioctl.h>
 
+#include <unicode/utypes.h>
+#include <unicode/ucnv.h>
+#include <unicode/ucnv_err.h>
+
 #include "sqlite3_exception.h"
 #include "sqlcipher_loading.h"
 
@@ -104,6 +108,69 @@ int native_status(JNIEnv* env, jobject object, jint operation, jboolean reset)
   return value;
 }
 
+void native_key_char(JNIEnv* env, jobject object, jcharArray jKey)
+{
+  char *keyUtf8        = 0;
+  int lenUtf8          = 0;
+  jchar* keyUtf16      = 0;
+  jsize lenUtf16       = 0;
+  UErrorCode status    = U_ZERO_ERROR;
+  UConverter *encoding = 0;
+
+  sqlite3 * handle = (sqlite3 *)env->GetIntField(object, offset_db_handle);
+
+  keyUtf16 = env->GetCharArrayElements(jKey, 0);
+  lenUtf16 = env->GetArrayLength(jKey);
+
+  // no key, bailing out.
+  if ( lenUtf16 == 0 ) goto done;
+
+  encoding = ucnv_open("UTF-8", &status);
+  if( U_FAILURE(status) ) {
+    throw_sqlite3_exception(env, "native_key_char: opening encoding converter failed");
+    goto done;
+  }
+
+  lenUtf8 = ucnv_fromUChars(encoding, NULL, 0, keyUtf16, lenUtf16, &status);
+  status = (status == U_BUFFER_OVERFLOW_ERROR) ? U_ZERO_ERROR : status;
+  if( U_FAILURE(status) ) {
+    throw_sqlite3_exception(env, "native_key_char: utf8 length unknown");
+    goto done;
+  }
+
+  keyUtf8 = (char*) malloc(lenUtf8 * sizeof(char));
+  ucnv_fromUChars(encoding, keyUtf8, lenUtf8, keyUtf16, lenUtf16, &status);
+  if( U_FAILURE(status) ) {
+    throw_sqlite3_exception(env, "native_key_char: utf8 conversion failed");
+    goto done;
+  }
+
+  if ( sqlite3_key(handle, keyUtf8, lenUtf8) != SQLITE_OK ) {
+    throw_sqlite3_exception(env, handle);
+  }
+
+done:
+  env->ReleaseCharArrayElements(jKey, keyUtf16, 0);
+  if(encoding != 0) ucnv_close(encoding);
+  if(keyUtf8 != 0)  free(keyUtf8);
+}
+
+void native_key_str(JNIEnv* env, jobject object, jstring jKey)
+{
+  sqlite3 * handle = (sqlite3 *)env->GetIntField(object, offset_db_handle);
+
+  char const * key = env->GetStringUTFChars(jKey, NULL);
+  jsize keyLen = env->GetStringUTFLength(jKey);
+
+  if ( keyLen > 0 ) {
+    int status = sqlite3_key(handle, key, keyLen);
+    if ( status != SQLITE_OK ) {
+        throw_sqlite3_exception(env, handle);
+    }
+  }
+  env->ReleaseStringUTFChars(jKey, key);
+}
+
 void native_rawExecSQL(JNIEnv* env, jobject object, jstring sql)
 {
     sqlite3 * handle = (sqlite3 *)env->GetIntField(object, offset_db_handle);
@@ -492,6 +559,8 @@ static JNINativeMethod sMethods[] =
     {"setICURoot", "(Ljava/lang/String;)V", (void *)setICURoot},
     {"native_rawExecSQL", "(Ljava/lang/String;)V", (void *)native_rawExecSQL},
     {"native_status", "(IZ)I", (void *)native_status},
+    {"native_key", "([C)V", (void *)native_key_char},
+    {"native_key", "(Ljava/lang/String;)V", (void *)native_key_str},
 };
 
 int register_android_database_SQLiteDatabase(JNIEnv *env)

src/net/sqlcipher/database/SQLiteDatabase.java

@@ -77,7 +77,7 @@ public class SQLiteDatabase extends SQLiteClosable {
     public int status(int operation, boolean reset){
         return native_status(operation, reset);
     }
-    
+
     public static void upgradeDatabaseFormatFromVersion1To2(File databaseToMigrate, String password) throws Exception {
 
         File newDatabasePath = null;
@@ -90,7 +90,7 @@ public void postKey(SQLiteDatabase database){
             database.execSQL("PRAGMA cipher_default_use_hmac = on");
           }
         };
- 
+
         try {
             newDatabasePath = File.createTempFile("temp", "db", databaseToMigrate.getParentFile());
             SQLiteDatabase source = SQLiteDatabase.openOrCreateDatabase(databaseToMigrate, password, null, hook);
@@ -920,15 +920,15 @@ public static SQLiteDatabase openDatabase(String path, String password, CursorFa
                                                            new WeakReference<SQLiteDatabase>(sqliteDatabase));
         return sqliteDatabase;
     }
-    
+
     public static SQLiteDatabase openOrCreateDatabase(File file, String password, CursorFactory factory, SQLiteDatabaseHook databaseHook){
         return openOrCreateDatabase(file.getPath(), password, factory, databaseHook);
     }
 
     public static SQLiteDatabase openOrCreateDatabase(String path, String password, CursorFactory factory, SQLiteDatabaseHook databaseHook) {
         return openDatabase(path, password, factory, CREATE_IF_NECESSARY, databaseHook);
     }
-    
+
     /**
      * Equivalent to openDatabase(file.getPath(), factory, CREATE_IF_NECESSARY).
      */
@@ -939,7 +939,7 @@ public static SQLiteDatabase openOrCreateDatabase(File file, String password, Cu
     /**
      * Equivalent to openDatabase(path, factory, CREATE_IF_NECESSARY).
      */
-    
+
     public static SQLiteDatabase openOrCreateDatabase(String path, String password, CursorFactory factory) {
         return openDatabase(path, password, factory, CREATE_IF_NECESSARY, null);
     }
@@ -968,7 +968,7 @@ public static SQLiteDatabase create(CursorFactory factory, String password) {
      * Close the database.
      */
     public void close() {
-        
+
         if (!isOpen()) {
             return; // already closed
         }
@@ -1496,7 +1496,7 @@ public Cursor rawQuery(String sql, String[] selectionArgs,
         SQLiteCursor c = (SQLiteCursor)rawQueryWithFactory(
                                                            null, sql, selectionArgs, null);
         c.setLoadStyle(initialRead, maxRead);
-        
+
         return c;
     }
 
@@ -1875,7 +1875,7 @@ public void rawExecSQL(String sql){
             logTimeStat(sql, timeStart, null);
         }
     }
-    
+
     /**
      * Execute a single SQL statement that is not a query. For example, CREATE
      * TABLE, DELETE, INSERT, etc. Multiple statements separated by ;s are not
@@ -1929,7 +1929,7 @@ protected void finalize() {
     public SQLiteDatabase(String path, String password, CursorFactory factory, int flags) {
         this(path, password, factory, flags, null);
     }
-    
+
     /**
      * Private constructor. See {@link #create} and {@link #openDatabase}.
      *
@@ -1949,12 +1949,12 @@ public SQLiteDatabase(String path, String password, CursorFactory factory, int f
         mStackTrace = new DatabaseObjectNotClosedException().fillInStackTrace();
         mFactory = factory;
         dbopen(mPath, mFlags);
-        
+
         if(databaseHook != null){
             databaseHook.preKey(this);
         }
-        
-        execSQL("PRAGMA key = '" + password + "'");
+
+        native_key(password.toCharArray());
 
         if(databaseHook != null){
             databaseHook.postKey(this);
@@ -2370,7 +2370,7 @@ private static ArrayList<Pair<String, String>> getAttachedDbs(SQLiteDatabase dbO
      * Sets the root directory to search for the ICU data file
      */
     public static native void setICURoot(String path);
-    
+
     /**
      * Native call to open the database.
      *
@@ -2435,4 +2435,7 @@ private static ArrayList<Pair<String, String>> getAttachedDbs(SQLiteDatabase dbO
     private native void native_rawExecSQL(String sql);
 
     private native int native_status(int operation, boolean reset);
+
+    private native void native_key(char[] key) throws SQLException;
+    private native void native_key(String key) throws SQLException;
 }