Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4039dc49 authored by Jeff Sharkey's avatar Jeff Sharkey Committed by Jeff Sharkey
Browse files

Execute "strict" queries with extra parentheses. 

SQLiteQueryBuilder has a setStrict() mode which can be used to
detect SQL attacks from untrusted sources, which it does by running
each query twice: once with an extra set of parentheses, and if that
succeeds, it runs the original query verbatim.

This sadly doesn't catch inputs of the type "1=1) OR (1=1", which
creates valid statements for both tests above, but the final executed
query ends up leaking data due to SQLite operator precedence.

Instead, we need to continue compiling both variants, but we need
to execute the query with the additional parentheses to ensure
data won't be leaked.

Test: atest cts/tests/tests/database/src/android/database/sqlite/cts/SQLiteQueryBuilderTest.java
Bug: 111085900
Merged-In: Ie85a95003ae134eef2fdfbf074c2f82d0a6a9f26
Change-Id: Ie85a95003ae134eef2fdfbf074c2f82d0a6a9f26
parent 55777e52

core/java/android/database/sqlite/SQLiteQueryBuilder.java

+25 −9
Original line number Diff line number Diff line
@@ -579,6 +579,16 @@ public class SQLiteQueryBuilder { 
            queryArgs = Bundle.EMPTY;

        }



        // Final SQL that we will execute

        final String sql;



        final String unwrappedSql = buildQuery(projection,

                queryArgs.getString(QUERY_ARG_SQL_SELECTION),

                queryArgs.getString(QUERY_ARG_SQL_GROUP_BY),

                queryArgs.getString(QUERY_ARG_SQL_HAVING),

                queryArgs.getString(QUERY_ARG_SQL_SORT_ORDER),

                queryArgs.getString(QUERY_ARG_SQL_LIMIT));



        if (mStrict) {

            // Validate the user-supplied selection to detect syntactic anomalies

            // in the selection string that could indicate a SQL injection attempt.
@@ -588,23 +598,29 @@ public class SQLiteQueryBuilder { 
            // would escape the SQL expression while maintaining balanced parentheses

            // in both the wrapped and original forms.



            // NOTE: The ordering of the below operations is important; we must

            // execute the wrapped query to ensure the untrusted clause has been

            // fully isolated.



            // TODO: decode SORT ORDER and LIMIT clauses, since they can contain

            // "expr" inside that need to be validated

            final String sql = buildQuery(projection,



            final String wrappedSql = buildQuery(projection,

                    wrap(queryArgs.getString(QUERY_ARG_SQL_SELECTION)),

                    queryArgs.getString(QUERY_ARG_SQL_GROUP_BY),

                    queryArgs.getString(QUERY_ARG_SQL_HAVING),

                    queryArgs.getString(QUERY_ARG_SQL_SORT_ORDER),

                    queryArgs.getString(QUERY_ARG_SQL_LIMIT));

            db.validateSql(sql, cancellationSignal); // will throw if query is invalid

        }



        final String sql = buildQuery(projection,

                queryArgs.getString(QUERY_ARG_SQL_SELECTION),

                queryArgs.getString(QUERY_ARG_SQL_GROUP_BY),

                queryArgs.getString(QUERY_ARG_SQL_HAVING),

                queryArgs.getString(QUERY_ARG_SQL_SORT_ORDER),

                queryArgs.getString(QUERY_ARG_SQL_LIMIT));

            // Validate the unwrapped query

            db.validateSql(unwrappedSql, cancellationSignal);



            // Execute wrapped query for extra protection

            sql = wrappedSql;

        } else {

            // Execute unwrapped query

            sql = unwrappedSql;

        }



        final String[] sqlArgs = ArrayUtils.concat(String.class,

                queryArgs.getStringArray(QUERY_ARG_SQL_SELECTION_ARGS), mWhereArgs);