8000 Fix potential SQL injection when keying the database · Sjith/android-database-sqlcipher@2c113f6 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2c113f6

Browse files
abeluckabeluck
committed
Fix potential SQL injection when keying the database
SQLite doesn't support compiled statements with PRAGMA values, and manually escaping the string seemed chancy. So, I opted to call the API function directly.
1 parent 261b760 commit 2c113f6

File tree

3 files changed

+92
-15
lines changed

3 files changed

+92
-15
lines changed

‎jni/Android.mk

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ endif
1616

1717
include $(CLEAR_VARS)
1818

19+
# expose the sqlcipher C API
20+
LOCAL_CFLAGS += -DSQLITE_HAS_CODEC
21+
1922
LOCAL_SRC_FILES:= \
2023
net_sqlcipher_database_SQLiteCompiledSql.cpp \
2124
net_sqlcipher_database_SQLiteDatabase.cpp \
@@ -37,6 +40,7 @@ LOCAL_C_INCLUDES += \
3740
$(EXTERNAL_PATH)/platform-system-core/include \
3841
$(LOCAL_PATH)/include \
3942
$(EXTERNAL_PATH)/platform-frameworks-base/include \
43+
$(EXTERNAL_PATH)/icu4c/common \
4044

4145
LOCAL_SHARED_LIBRARIES := \
4246
libcrypto \
@@ -53,8 +57,9 @@ LOCAL_LDLIBS += -ldl -llog
5357
LOCAL_LDLIBS += -lnativehelper -landroid_runtime -lutils -lbinder
5458
# these are build in the ../external section
5559

56-
#LOCAL_REQUIRED_MODULES += libsqlcipher libicuuc libicui18n
57-
LOCAL_LDLIBS += -lsqlcipher_android
60+
LOCAL_LDLIBS += -lsqlcipher_android
61+
LOCAL_LDFLAGS += -L../obj/local/$(TARGET_ARCH_ABI)
62+
LOCAL_LDLIBS += -licui18n -licuuc
5863

5964
ifeq ($(WITH_MALLOC_LEAK_CHECK),true)
6065
LOCAL_CFLAGS += -DMALLOC_LEAK_CHECK

‎jni/net_sqlcipher_database_SQLiteDatabase.cpp

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@
3737
#include <string.h>
3838
#include <sys/ioctl.h>
3939

40+
#include <unicode/utypes.h>
41+
#include <unicode/ucnv.h>
42+
#include <unicode/ucnv_err.h>
43+
4044
#include "sqlite3_exception.h"
4145
#include "sqlcipher_loading.h"
4246

@@ -104,6 +108,69 @@ int native_status(JNIEnv* env, jobject object, jint operation, jboolean reset)
104108
return value;
105109
}
106110

111+
void native_key_char(JNIEnv* env, jobject object, jcharArray jKey)
112+
{
113+
char *keyUtf8 = 0;
114+
int lenUtf8 = 0;
115+
jchar* keyUtf16 = 0;
116+
jsize lenUtf16 = 0;
117+
UErrorCode status = U_ZERO_ERROR;
118+
UConverter *encoding = 0;
119+
120+
sqlite3 * handle = (sqlite3 *)env->GetIntField(object, offset_db_handle);
121+
122+
keyUtf16 = env->GetCharArrayElements(jKey, 0);
123+
lenUtf16 = env->GetArrayLength(jKey);
124+
125+
// no key, bailing out.
126+
if ( lenUtf16 == 0 ) goto done;
127+
128+
encoding = ucnv_open("UTF-8", &status);
129+
if( U_FAILURE(status) ) {
130+
throw_sqlite3_exception(env, "native_key_char: opening encoding converter failed");
131+
goto done;
132+
}
133+
134+
lenUtf8 = ucnv_fromUChars(encoding, NULL, 0, keyUtf16, lenUtf16, &status);
135+
status = (status == U_BUFFER_OVERFLOW_ERROR) ? U_ZERO_ERROR : status;
136+
if( U_FAILURE(status) ) {
137+
throw_sqlite3_exception(env, "native_key_char: utf8 length unknown");
138+
goto done;
139+
}
140+
141+
keyUtf8 = (char*) malloc(lenUtf8 * sizeof(char));
142+
ucnv_fromUChars(encoding, keyUtf8, lenUtf8, keyUtf16, lenUtf16, &status);
143+
if( U_FAILURE(status) ) {
144+
throw_sqlite3_exception(env, "native_key_char: utf8 conversion failed");
145+
goto done;
146+
}
147+
148+
if ( sqlite3_key(handle, keyUtf8, lenUtf8) != SQLITE_OK ) {
149+
throw_sqlite3_exception(env, handle);
150+
}
151+
152+
done:
153+
env->ReleaseCharArrayElements(jKey, keyUtf16, 0);
154+
if(encoding != 0) ucnv_close(encoding);
155+
if(keyUtf8 != 0) free(keyUtf8);
156+
}
157+
158+
void native_key_str(JNIEnv* env, jobject object, jstring jKey)
159+
{
160+
sqlite3 * handle = (sqlite3 *)env->GetIntField(object, offset_db_handle);
161+
162+
char const * key = env->GetStringUTFChars(jKey, NULL);
163+
jsize keyLen = env->GetStringUTFLength(jKey);
164+
165+
if ( keyLen > 0 ) {
166+
int status = sqlite3_key(handle, key, keyLen);
167+
if ( status != SQLITE_OK ) {
168+
throw_sqlite3_exception(env, handle);
169+
}
170+
}
171+
env->ReleaseStringUTFChars(jKey, key);
172+
}
173+
107174
void native_rawExecSQL(JNIEnv* env, jobject object, jstring sql)
108175
{
109176
sqlite3 * handle = (sqlite3 *)env->GetIntField(object, offset_db_handle);
@@ -492,6 +559,8 @@ static JNINativeMethod sMethods[] =
492559
{"setICURoot", "(Ljava/lang/String;)V", (void *)setICURoot},
493560
{"native_rawExecSQL", "(Ljava/lang/String;)V", (void *)native_rawExecSQL},
494561
{"native_status", "(IZ)I", (void *)native_status},
562+
{"native_key", "([C)V", (void *)native_key_char},
563+
{"native_key", "(Ljava/lang/String;)V", (void *)native_key_str},
495564
};
496565

497566
int register_android_database_SQLiteDatabase(JNIEnv *env)

‎src/net/sqlcipher/database/SQLiteDatabase.java

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public class SQLiteDatabase extends SQLiteClosable {
7777
public int status(int operation, boolean reset){
7878
return native_status(operation, reset);
7979
}
80-
80+
8181
public static void upgradeDatabaseFormatFromVersion1To2(File databaseToMigrate, String password) throws Exception {
8282

8383
File newDatabasePath = null;
@@ -90,7 +90,7 @@ public void postKey(SQLiteDatabase database){
9090
database.execSQL("PRAGMA cipher_default_use_hmac = on");
9191
}
9292
};
93-
93+
9494
try {
9595
newDatabasePath = File.createTempFile("temp", "db", databaseToMigrate.getParentFile());
9696
SQLiteDatabase source = SQLiteDatabase.openOrCreateDatabase(databaseToMigrate, password, null, hook);
@@ -920,15 +920,15 @@ public static SQLiteDatabase openDatabase(String path, String password, CursorFa
920920
new WeakReference<SQLiteDatabase>(sqliteDatabase));
921921
return sqliteDatabase;
922922
}
923-
923+
924924
public static SQLiteDatabase openOrCreateDatabase(File file, String password, CursorFactory factory, SQLiteDatabaseHook databaseHook){
925925
return openOrCreateDatabase(file.getPath(), password, factory, databaseHook);
926926
}
927927

928928
public static SQLiteDatabase openOrCreateDatabase(String path, String password, CursorFactory factory, SQLiteDatabaseHook databaseHook) {
929929
return openDatabase(path, password, factory, CREATE_IF_NECESSARY, databaseHook);
930930
}
931-
931+
932932
/**
933933
* Equivalent to openDatabase(file.getPath(), factory, CREATE_IF_NECESSARY).
934934
*/
@@ -939,7 +939,7 @@ public static SQLiteDatabase openOrCreateDatabase(File file, String password, Cu
939939
/**
940940
* Equivalent to openDatabase(path, factory, CREATE_IF_NECESSARY).
941941
*/
942-
942+
943943
public static SQLiteDatabase openOrCreateDatabase(String path, String password, CursorFactory factory) {
944944
return openDatabase(path, password, factory, CREATE_IF_NECESSARY, null);
945945
}
@@ -968,7 +968,7 @@ public static SQLiteDatabase create(CursorFactory factory, String password) {
968968
* Close the database.
969969
*/
970970
public void close() {
971-
971+
972972
if (!isOpen()) {
973973
return; // already closed
974974
}
@@ -1496,7 +1496,7 @@ public Cursor rawQuery(String sql, String[] selectionArgs,
14961496
SQLiteCursor c = (SQLiteCursor)rawQueryWithFactory(
14971497
null, sql, selectionArgs, null);
14981498
c.setLoadStyle(initialRead, maxRead);
1499-
1499+
15001500
return c;
15011501
}
15021502

@@ -1875,7 +1875,7 @@ public void rawExecSQL(String sql){
18751875
logTimeStat(sql, timeStart, null);
18761876
}
18771877
}
1878-
1878+
18791879
/**
18801880
* Execute a single SQL statement that is not a query. For example, CREATE
18811881
* TABLE, DELETE, INSERT, etc. Multiple statements separated by ;s are not
@@ -1929,7 +1929,7 @@ protected void finalize() {
19291929
public SQLiteDatabase(String path, String password, CursorFactory factory, int flags) {
19301930
this(path, password, factory, flags, null);
19311931
}
1932-
1932+
19331933
/**
19341934
* Private constructor. See {@link #create} and {@link #openDatabase}.
19351935
*
@@ -1949,12 +1949,12 @@ public SQLiteDatabase(String path, String password, CursorFactory factory, int f
19491949
mStackTrace = new DatabaseObjectNotClosedException().fillInStackTrace();
19501950
mFactory = factory;
19511951
dbopen(mPath, mFlags);
1952-
1952+
19531953
if(databaseHook != null){
19541954
databaseHook.preKey(this);
19551955
}
1956-
1957-
execSQL("PRAGMA key = '" + password + "'");
1956+
1957+
native_key(password.toCharArray());
19581958

19591959
if(databaseHook != null){
19601960
databaseHook.postKey(this);
@@ -2370,7 +2370,7 @@ private static ArrayList<Pair<String, String>> getAttachedDbs(SQLiteDatabase dbO
23702370
* Sets the root directory to search for the ICU data file
23712371
*/
23722372
public static native void setICURoot(String path);
2373-
2373+
23742374
/**
23752375
* Native call to open the database.
23762376
*
@@ -2435,4 +2435,7 @@ private static ArrayList<Pair<String, String>> getAttachedDbs(SQLiteDatabase dbO
24352435
private native void native_rawExecSQL(String sql);
24362436

24372437
private native int native_status(int operation, boolean reset);
2438+
2439+
private native void native_key(char[] key) throws SQLException;
2440+
private native void native_key(String key) throws SQLException;
24382441
}

0 commit comments

Comments
 (0)
0