Skip to content

Fix potential SQL injection when keying the database #95

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 19, 2013
Merged

Fix potential SQL injection when keying the database #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions jni/Android.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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 \
Expand All @@ -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
Expand Down
69 changes: 69 additions & 0 deletions jni/net_sqlcipher_database_SQLiteDatabase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
29 changes: 16 additions & 13 deletions src/net/sqlcipher/database/SQLiteDatabase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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).
*/
Expand All @@ -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);
}
Expand Down Expand Up @@ -968,7 +968,7 @@ public static SQLiteDatabase create(CursorFactory factory, String password) {
* Close the database.
*/
public void close() {

if (!isOpen()) {
return; // already closed
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}.
*
Expand All @@ -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);
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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;
}