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

packages/WAPPushManager/src/com/android/smspush/WapPushManager.java

@@ -117,14 +117,18 @@ public class WapPushManager extends Service {
          */
         protected queryData queryLastApp(SQLiteDatabase db,
                 String app_id, String content_type) {
-            String sql = "select install_order, package_name, class_name, "
+            if (LOCAL_LOGV) Log.v(LOG_TAG, "queryLastApp app_id: " + app_id
-                    + " app_type, need_signature, further_processing"
+                    + " content_type: " +  content_type);
-                    + " from " + APPID_TABLE_NAME
+
-                    + " where x_wap_application=\'" + app_id + "\'"
+            Cursor cur = db.query(APPID_TABLE_NAME,
-                    + " and content_type=\'" + content_type + "\'"
+                    new String[] {"install_order", "package_name", "class_name",
-                    + " order by install_order desc";
+                    "app_type", "need_signature", "further_processing"},
-            if (DEBUG_SQL) Log.v(LOG_TAG, "sql: " + sql);
+                    "x_wap_application=? and content_type=?",
-            Cursor cur = db.rawQuery(sql, null);
+                    new String[] {app_id, content_type},
+                    null /* groupBy */,
+                    null /* having */,
+                    "install_order desc" /* orderBy */);
+
             queryData ret = null;
 
             if (cur.moveToNext()) {
@@ -392,10 +396,20 @@ public class WapPushManager extends Service {
         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

packages/WAPPushManager/tests/src/com/android/smspush/unitTests/WapPushTest.java

@@ -551,6 +551,25 @@ public class WapPushTest extends ServiceTestCase<WapPushManager> {
         mContentTypeValue = originalContentTypeValue;
     }
 
+    /**
+     * 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.
      */
@@ -1477,7 +1496,7 @@ public class WapPushTest extends ServiceTestCase<WapPushManager> {
         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;