SQL token checker to detect uses of hidden tables/columns

- Detect invalid SQL code (e.g. contains a semi-colon)
in not only WHERE for query() but in other places too.

- Disallow use of the word "select" and table/view names
in the supplied code to prevent subqueries.

- This mechanism will be used to hide columns in the futire too.

Test: adb shell am instrument -w com.android.providers.contacts.tests

Bug 31559073

Change-Id: Ib4293b4caf7e341186ee8bd4cc2d7dad7155c48d

4 files changed, 355 insertions, 21 deletions

diff --git a/tests/Android.mk b/tests/Android.mk
index 35a6b395..d2f99861 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -4,7 +4,9 @@ include $(CLEAR_VARS)
 # We only want this apk build for tests.
 LOCAL_MODULE_TAGS := tests
 
-LOCAL_STATIC_JAVA_LIBRARIES := mockito-target
+LOCAL_STATIC_JAVA_LIBRARIES := \
+    android-support-test \
+    mockito-target-minus-junit4
 
 LOCAL_JAVA_LIBRARIES := android.test.runner
 
diff --git a/tests/src/com/android/providers/contacts/SqlInjectionDetectionTest.java b/tests/src/com/android/providers/contacts/SqlInjectionDetectionTest.java
index e7b80a04..c7eb64cc 100644
--- a/tests/src/com/android/providers/contacts/SqlInjectionDetectionTest.java
+++ b/tests/src/com/android/providers/contacts/SqlInjectionDetectionTest.java
@@ -17,6 +17,7 @@
 package com.android.providers.contacts;
 
 import static com.android.providers.contacts.EvenMoreAsserts.assertThrows;
+import static com.android.providers.contacts.TestUtils.cv;
 
 import android.database.Cursor;
 import android.database.sqlite.SQLiteException;
@@ -25,6 +26,7 @@ import android.net.Uri.Builder;
 import android.provider.ContactsContract;
 import android.provider.ContactsContract.CommonDataKinds.Phone;
 import android.provider.ContactsContract.Contacts;
+import android.provider.ContactsContract.Data;
 import android.test.suitebuilder.annotation.MediumTest;
 
 import com.android.providers.contacts.testutil.RawContactUtil;
@@ -42,36 +44,48 @@ import com.android.providers.contacts.testutil.RawContactUtil;
 public class SqlInjectionDetectionTest extends BaseContactsProvider2Test {
     private static final String[] PHONE_ID_PROJECTION = new String[] { Phone._ID };
 
-    public void testPhoneQueryValid() {
-        long rawContactId = RawContactUtil.createRawContactWithName(mResolver, "Hot", "Tamale");
-        insertPhoneNumber(rawContactId, "555-123-4567");
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+
+        getContactsProvider().getContactsDatabaseHelperForTest().setSqlCheckEnabled(true);
+    }
 
+    public void testPhoneQueryValid() {
         assertQueryValid(Phone.CONTENT_URI, PHONE_ID_PROJECTION,
                 Phone.NUMBER + "='555-123-4567'", null);
     }
 
     public void testPhoneQueryBadProjection() {
-        long rawContactId = RawContactUtil.createRawContactWithName(mResolver, "Hot", "Tamale");
-        insertPhoneNumber(rawContactId, "555-123-4567");
-
-        assertQueryThrows(IllegalArgumentException.class, Phone.CONTENT_URI,
+        assertQueryThrows(Phone.CONTENT_URI,
                 new String[] { "0 UNION SELECT _id FROM view_data--" }, null, null);
+
+        // Invalid column names should be detected too.
+        assertQueryThrows(Phone.CONTENT_URI, new String[] { "a" }, null, null);
+        assertQueryThrows(Phone.CONTENT_URI, new String[] { " _id" }, null, null);
+
+        // This is still invalid because we only allow exact column names in projections.
+        assertQueryThrows(Phone.CONTENT_URI, new String[] { "[_id]" }, null, null);
     }
 
     public void testPhoneQueryBadSelection() {
-        long rawContactId = RawContactUtil.createRawContactWithName(mResolver, "Hot", "Tamale");
-        insertPhoneNumber(rawContactId, "555-123-4567");
-
-        assertQueryThrows(SQLiteException.class, Phone.CONTENT_URI, PHONE_ID_PROJECTION,
+        assertQueryThrows(Phone.CONTENT_URI, PHONE_ID_PROJECTION,
                 "0=1) UNION SELECT _id FROM view_data--", null);
+        assertQueryThrows(Phone.CONTENT_URI, PHONE_ID_PROJECTION, ";delete from contacts", null);
+        assertQueryThrows(Phone.CONTENT_URI, PHONE_ID_PROJECTION,
+                "_id in default_directory", null);
+        assertQueryThrows(Phone.CONTENT_URI, PHONE_ID_PROJECTION,
+                "_id in (select _id from default_directory)", null);
     }
 
     public void testPhoneQueryBadSortOrder() {
-        long rawContactId = RawContactUtil.createRawContactWithName(mResolver, "Hot", "Tamale");
-        insertPhoneNumber(rawContactId, "555-123-4567");
-
-        assertQueryThrows(SQLiteException.class, Phone.CONTENT_URI,
+        assertQueryThrows(Phone.CONTENT_URI,
                 PHONE_ID_PROJECTION, null, "_id UNION SELECT _id FROM view_data--");
+        assertQueryThrows(Phone.CONTENT_URI, PHONE_ID_PROJECTION, null, ";delete from contacts");
+        assertQueryThrows(Phone.CONTENT_URI, PHONE_ID_PROJECTION, null,
+                "_id in default_directory");
+        assertQueryThrows(Phone.CONTENT_URI, PHONE_ID_PROJECTION,
+                null, "exists (select _id from default_directory)");
     }
 
     public void testPhoneQueryBadLimit() {
@@ -100,14 +114,39 @@ public class SqlInjectionDetectionTest extends BaseContactsProvider2Test {
         c.close();
     }
 
-    private <T extends Exception> void assertQueryThrows(Class<T> exception, final Uri uri,
+    private <T extends Exception> void assertQueryThrows(final Uri uri,
             final String[] projection, final String selection, final String sortOrder) {
-        assertThrows(exception, new Runnable() {
-            @Override
-            public void run() {
+        assertThrows(IllegalArgumentException.class, () -> {
                 final Cursor c = mResolver.query(uri, projection, selection, null, sortOrder);
                 c.close();
-            }
+        });
+    }
+
+    public void testBadDelete() {
+        assertThrows(IllegalArgumentException.class, () -> {
+            mResolver.delete(Contacts.CONTENT_URI, ";delete from contacts;--", null);
+        });
+        assertThrows(IllegalArgumentException.class, () -> {
+            mResolver.delete(Contacts.CONTENT_URI, "_id in default_directory", null);
+        });
+    }
+
+    public void testBadUpdate() {
+        assertThrows(IllegalArgumentException.class, () -> {
+            mResolver.update(Data.CONTENT_URI, cv(), ";delete from contacts;--", null);
+        });
+        assertThrows(IllegalArgumentException.class, () -> {
+            mResolver.update(Data.CONTENT_URI, cv(), "_id in default_directory", null);
+        });
+        assertThrows(IllegalArgumentException.class, () -> {
+            mResolver.update(Data.CONTENT_URI, cv("_id/**/", 1), null, null);
+        });
+        mResolver.update(Data.CONTENT_URI, cv("[data1]", 1), null, null); // this is actually fine
+    }
+
+    public void testBadInsert() {
+        assertThrows(IllegalArgumentException.class, () -> {
+            mResolver.insert(Data.CONTENT_URI, cv("_id/**/", 1));
         });
     }
 }
diff --git a/tests/src/com/android/providers/contacts/sqlite/DatabaseAnalyzerTest.java b/tests/src/com/android/providers/contacts/sqlite/DatabaseAnalyzerTest.java
new file mode 100644
index 00000000..0ef020b7
--- /dev/null
+++ b/tests/src/com/android/providers/contacts/sqlite/DatabaseAnalyzerTest.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.providers.contacts.sqlite;
+
+import android.test.AndroidTestCase;
+
+import com.android.providers.contacts.ContactsDatabaseHelper;
+
+import java.util.List;
+
+public class DatabaseAnalyzerTest extends AndroidTestCase {
+    public void testFindTableViewsAllowingColumns() {
+        final ContactsDatabaseHelper dbh =
+                ContactsDatabaseHelper.getNewInstanceForTest(getContext());
+        try {
+            final List<String> list =  DatabaseAnalyzer.findTableViewsAllowingColumns(
+                    dbh.getReadableDatabase());
+
+            assertTrue(list.contains("contacts"));
+            assertTrue(list.contains("raw_contacts"));
+            assertTrue(list.contains("view_contacts"));
+            assertTrue(list.contains("view_raw_contacts"));
+            assertTrue(list.contains("view_data"));
+
+            assertFalse(list.contains("data"));
+            assertFalse(list.contains("_id"));
+
+        } finally {
+            dbh.close();
+        }
+    }
+}
\ No newline at end of file
diff --git a/tests/src/com/android/providers/contacts/sqlite/SqlCheckerTest.java b/tests/src/com/android/providers/contacts/sqlite/SqlCheckerTest.java
new file mode 100644
index 00000000..a0a8622c
--- /dev/null
+++ b/tests/src/com/android/providers/contacts/sqlite/SqlCheckerTest.java
@@ -0,0 +1,247 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+package com.android.providers.contacts.sqlite;
+
+import android.test.AndroidTestCase;
+import android.test.MoreAsserts;
+
+import com.android.providers.contacts.sqlite.SqlChecker.InvalidSqlException;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+public class SqlCheckerTest extends AndroidTestCase {
+    private ArrayList<String> getTokens(String sql) {
+        final ArrayList<String> tokens = new ArrayList<>();
+
+        SqlChecker.findTokens(sql, SqlChecker.OPTION_NONE,  token -> tokens.add(token));
+
+        return tokens;
+    }
+
+    private void checkTokens(String sql, String spaceSeparatedExpectedTokens) {
+        final List<String> expected = spaceSeparatedExpectedTokens == null
+                ? new ArrayList<>()
+                : Arrays.asList(spaceSeparatedExpectedTokens.split(" +"));
+
+        assertEquals(expected, getTokens(sql));
+    }
+
+    private void assertInvalidSql(String sql, String message) {
+        try {
+            getTokens(sql);
+            fail("Didn't throw InvalidSqlException");
+        } catch (InvalidSqlException e) {
+            MoreAsserts.assertContainsRegex(message, e.getMessage());
+        }
+    }
+
+    public void testWhitespaces() {
+        checkTokens("  select  \t\r\n a\n\n  ", "select a");
+        checkTokens("a b", "a b");
+    }
+
+    public void testComment() {
+        checkTokens("--\n", null);
+        checkTokens("a--\n", "a");
+        checkTokens("a--abcdef\n", "a");
+        checkTokens("a--abcdef\nx", "a x");
+        checkTokens("a--\nx", "a x");
+        assertInvalidSql("a--abcdef", "Unterminated comment");
+        assertInvalidSql("a--abcdef\ndef--", "Unterminated comment");
+
+        checkTokens("/**/", null);
+        assertInvalidSql("/*", "Unterminated comment");
+        assertInvalidSql("/*/", "Unterminated comment");
+        assertInvalidSql("/*\n* /*a", "Unterminated comment");
+        checkTokens("a/**/", "a");
+        checkTokens("/**/b", "b");
+        checkTokens("a/**/b", "a b");
+        checkTokens("a/* -- \n* /* **/b", "a b");
+    }
+
+    public void testStrings() {
+        assertInvalidSql("'", "Unterminated quote");
+        assertInvalidSql("a'", "Unterminated quote");
+        assertInvalidSql("a'''", "Unterminated quote");
+        assertInvalidSql("a''' ", "Unterminated quote");
+        checkTokens("''", null);
+        checkTokens("''''", null);
+        checkTokens("a''''b", "a b");
+        checkTokens("a' '' 'b", "a b");
+        checkTokens("'abc'", null);
+        checkTokens("'abc\ndef'", null);
+        checkTokens("a'abc\ndef'", "a");
+        checkTokens("'abc\ndef'b", "b");
+        checkTokens("a'abc\ndef'b", "a b");
+        checkTokens("a'''abc\nd''ef'''b", "a b");
+    }
+
+    public void testDoubleQuotes() {
+        assertInvalidSql("\"", "Unterminated quote");
+        assertInvalidSql("a\"", "Unterminated quote");
+        assertInvalidSql("a\"\"\"", "Unterminated quote");
+        assertInvalidSql("a\"\"\" ", "Unterminated quote");
+        checkTokens("\"\"", "");
+        checkTokens("\"\"\"\"", "\"");
+        checkTokens("a\"\"\"\"b", "a \" b");
+        checkTokens("a\"\t\"\"\t\"b", "a  \t\"\t  b");
+        checkTokens("\"abc\"", "abc");
+        checkTokens("\"abc\ndef\"", "abc\ndef");
+        checkTokens("a\"abc\ndef\"", "a abc\ndef");
+        checkTokens("\"abc\ndef\"b", "abc\ndef b");
+        checkTokens("a\"abc\ndef\"b", "a abc\ndef b");
+        checkTokens("a\"\"\"abc\nd\"\"ef\"\"\"b", "a \"abc\nd\"ef\" b");
+    }
+
+    public void testBackQuotes() {
+        assertInvalidSql("`", "Unterminated quote");
+        assertInvalidSql("a`", "Unterminated quote");
+        assertInvalidSql("a```", "Unterminated quote");
+        assertInvalidSql("a``` ", "Unterminated quote");
+        checkTokens("``", "");
+        checkTokens("````", "`");
+        checkTokens("a````b", "a ` b");
+        checkTokens("a`\t``\t`b", "a  \t`\t  b");
+        checkTokens("`abc`", "abc");
+        checkTokens("`abc\ndef`", "abc\ndef");
+        checkTokens("a`abc\ndef`", "a abc\ndef");
+        checkTokens("`abc\ndef`b", "abc\ndef b");
+        checkTokens("a`abc\ndef`b", "a abc\ndef b");
+        checkTokens("a```abc\nd``ef```b", "a `abc\nd`ef` b");
+    }
+
+    public void testBrackets() {
+        assertInvalidSql("[", "Unterminated quote");
+        assertInvalidSql("a[", "Unterminated quote");
+        assertInvalidSql("a[ ", "Unterminated quote");
+        assertInvalidSql("a[[ ", "Unterminated quote");
+        checkTokens("[]", "");
+        checkTokens("[[]", "[");
+        checkTokens("a[[]b", "a [ b");
+        checkTokens("a[\t[\t]b", "a  \t[\t  b");
+        checkTokens("[abc]", "abc");
+        checkTokens("[abc\ndef]", "abc\ndef");
+        checkTokens("a[abc\ndef]", "a abc\ndef");
+        checkTokens("[abc\ndef]b", "abc\ndef b");
+        checkTokens("a[abc\ndef]b", "a abc\ndef b");
+        checkTokens("a[[abc\nd[ef[]b", "a [abc\nd[ef[ b");
+    }
+
+    public void testSemicolons() {
+        assertInvalidSql(";", "Semicolon is not allowed");
+        assertInvalidSql("  ;", "Semicolon is not allowed");
+        assertInvalidSql(";  ", "Semicolon is not allowed");
+        assertInvalidSql("-;-", "Semicolon is not allowed");
+        checkTokens("--;\n", null);
+        checkTokens("/*;*/", null);
+        checkTokens("';'", null);
+        checkTokens("[;]", ";");
+        checkTokens("`;`", ";");
+    }
+
+    public void testTokens() {
+        checkTokens("a,abc,a00b,_1,_123,abcdef", "a abc a00b _1 _123 abcdef");
+        checkTokens("a--\nabc/**/a00b''_1'''ABC'''`_123`abc[d]\"e\"f",
+                "a abc a00b _1 _123 abc d e f");
+    }
+
+    private SqlChecker getChecker(String... tokens) {
+        return new SqlChecker(Arrays.asList(tokens));
+    }
+
+    private void checkEnsureNoInvalidTokens(boolean ok, String sql, String... tokens) {
+        if (ok) {
+            getChecker(tokens).ensureNoInvalidTokens(sql);
+        } else {
+            try {
+                getChecker(tokens).ensureNoInvalidTokens(sql);
+                fail("Should have thrown");
+            } catch (InvalidSqlException e) {
+                // okay
+            }
+        }
+    }
+
+    public void testEnsureNoInvalidTokens() {
+        checkEnsureNoInvalidTokens(true, "a b c", "Select");
+
+        checkEnsureNoInvalidTokens(false, "a b ;c", "Select");
+        checkEnsureNoInvalidTokens(false, "a b seLeCt", "Select");
+
+        checkEnsureNoInvalidTokens(true, "a b select", "x");
+
+        checkEnsureNoInvalidTokens(false, "A b select", "x", "a");
+        checkEnsureNoInvalidTokens(false, "A b select", "a", "x");
+
+        checkEnsureNoInvalidTokens(true, "a /*select*/ b c ", "select");
+        checkEnsureNoInvalidTokens(true, "a 'select' b c ", "select");
+
+        checkEnsureNoInvalidTokens(true, "a b ';' c");
+        checkEnsureNoInvalidTokens(true, "a b /*;*/ c");
+    }
+
+    private void checkEnsureSingleTokenOnly(boolean ok, String sql, String... tokens) {
+        if (ok) {
+            getChecker(tokens).ensureSingleTokenOnly(sql);
+        } else {
+            try {
+                getChecker(tokens).ensureSingleTokenOnly(sql);
+                fail("Should have thrown");
+            } catch (InvalidSqlException e) {
+                // okay
+            }
+        }
+    }
+
+    public void testEnsureSingleTokenOnly() {
+        checkEnsureSingleTokenOnly(true, "a", "select");
+        checkEnsureSingleTokenOnly(true, "ab", "select");
+        checkEnsureSingleTokenOnly(true, "selec", "select");
+        checkEnsureSingleTokenOnly(true, "selectx", "select");
+
+        checkEnsureSingleTokenOnly(false, "select", "select");
+        checkEnsureSingleTokenOnly(false, "select", "a", "select");
+        checkEnsureSingleTokenOnly(false, "select", "select", "b");
+        checkEnsureSingleTokenOnly(false, "select", "a", "select", "b");
+
+
+        checkEnsureSingleTokenOnly(true, "`a`", "select");
+        checkEnsureSingleTokenOnly(true, "[a]", "select");
+        checkEnsureSingleTokenOnly(true, "\"a\"", "select");
+
+        checkEnsureSingleTokenOnly(false, "'a'", "select");
+
+        checkEnsureSingleTokenOnly(false, "b`a`", "select");
+        checkEnsureSingleTokenOnly(false, "b[a]", "select");
+        checkEnsureSingleTokenOnly(false, "b\"a\"", "select");
+        checkEnsureSingleTokenOnly(false, "b'a'", "select");
+
+        checkEnsureSingleTokenOnly(false, "`a`c", "select");
+        checkEnsureSingleTokenOnly(false, "[a]c", "select");
+        checkEnsureSingleTokenOnly(false, "\"a\"c", "select");
+        checkEnsureSingleTokenOnly(false, "'a'c", "select");
+
+        checkEnsureSingleTokenOnly(false, "", "select");
+        checkEnsureSingleTokenOnly(false, "--", "select");
+        checkEnsureSingleTokenOnly(false, "/**/", "select");
+        checkEnsureSingleTokenOnly(false, "  \n", "select");
+        checkEnsureSingleTokenOnly(false, "a--", "select");
+        checkEnsureSingleTokenOnly(false, "a/**/", "select");
+        checkEnsureSingleTokenOnly(false, "a  \n", "select");
+    }
+}