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

2 files changed, 42 insertions, 9 deletions

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 @@ 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, "
-                    + " 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 @@ 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
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 f436cb4..5ae6589 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 @@ public class WapPushTest extends ServiceTestCase<WapPushManager> {
     }
 
     /**
+     * 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 @@ 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;