Externally Reported Moderate Security Issue: SQL Injection in WAPPushManager
Bug 17969135
Use query (instead of rawQuery) and pass in arguments instead of building
the query with a giant string. Add a unit test that fails with the old
code but passes with the new code.
Change-Id: Id04a1db6fb95fcd923e1f36f5ab3b94402590918
diff --git a/packages/WAPPushManager/src/com/android/smspush/WapPushManager.java b/packages/WAPPushManager/src/com/android/smspush/WapPushManager.java
index 96e0377..e970367 100644
--- a/packages/WAPPushManager/src/com/android/smspush/WapPushManager.java
+++ b/packages/WAPPushManager/src/com/android/smspush/WapPushManager.java
@@ -117,14 +117,18 @@
*/
protected queryData queryLastApp(SQLiteDatabase db,
String app_id, String content_type) {
- String sql = "select install_order, package_name, class_name, "
- + " app_type, need_signature, further_processing"
- + " from " + APPID_TABLE_NAME
- + " where x_wap_application=\'" + app_id + "\'"
- + " and content_type=\'" + content_type + "\'"
- + " order by install_order desc";
- if (DEBUG_SQL) Log.v(LOG_TAG, "sql: " + sql);
- Cursor cur = db.rawQuery(sql, null);
+ if (LOCAL_LOGV) Log.v(LOG_TAG, "queryLastApp app_id: " + app_id
+ + " content_type: " + content_type);
+
+ Cursor cur = db.query(APPID_TABLE_NAME,
+ new String[] {"install_order", "package_name", "class_name",
+ "app_type", "need_signature", "further_processing"},
+ "x_wap_application=? and content_type=?",
+ new String[] {app_id, content_type},
+ null /* groupBy */,
+ null /* having */,
+ "install_order desc" /* orderBy */);
+
queryData ret = null;
if (cur.moveToNext()) {
@@ -392,10 +396,20 @@
SQLiteDatabase db = dbh.getReadableDatabase();
WapPushManDBHelper.queryData lastapp = dbh.queryLastApp(db, x_app_id, content_type);
+ if (LOCAL_LOGV) Log.v(LOG_TAG, "verifyData app id: " + x_app_id + " content type: " +
+ content_type + " lastapp: " + lastapp);
+
db.close();
if (lastapp == null) return false;
+ if (LOCAL_LOGV) Log.v(LOG_TAG, "verifyData lastapp.packageName: " + lastapp.packageName +
+ " lastapp.className: " + lastapp.className +
+ " lastapp.appType: " + lastapp.appType +
+ " lastapp.needSignature: " + lastapp.needSignature +
+ " lastapp.furtherProcessing: " + lastapp.furtherProcessing);
+
+
if (lastapp.packageName.equals(package_name)
&& lastapp.className.equals(class_name)
&& lastapp.appType == app_type
diff --git a/packages/WAPPushManager/tests/src/com/android/smspush/unitTests/WapPushTest.java b/packages/WAPPushManager/tests/src/com/android/smspush/unitTests/WapPushTest.java
index 305ee37..f7afc57 100644
--- a/packages/WAPPushManager/tests/src/com/android/smspush/unitTests/WapPushTest.java
+++ b/packages/WAPPushManager/tests/src/com/android/smspush/unitTests/WapPushTest.java
@@ -552,6 +552,25 @@
}
/**
+ * Add sqlite injection test
+ */
+ public void testAddPackage0() {
+ String inject = "' union select 0,'com.android.settings','com.android.settings.Settings',0,0,0--";
+
+ // insert new data
+ IWapPushManager iwapman = getInterface();
+ try {
+ assertFalse(iwapman.addPackage(
+ inject,
+ Integer.toString(mContentTypeValue),
+ mPackageName, mClassName,
+ WapPushManagerParams.APP_TYPE_SERVICE, true, true));
+ } catch (RemoteException e) {
+ assertTrue(false);
+ }
+ }
+
+ /**
* Add duprecated package test.
*/
public void testAddPackage2() {
@@ -1477,7 +1496,7 @@
System.arraycopy(mWspHeader, 0, array,
mGsmHeader.length + mUserDataHeader.length, mWspHeader.length);
System.arraycopy(mMessageBody, 0, array,
- mGsmHeader.length + mUserDataHeader.length + mWspHeader.length,
+ mGsmHeader.length + mUserDataHeader.length + mWspHeader.length,
mMessageBody.length);
return array;
