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

Commit 0e66ea6f authored by Jeff Sharkey's avatar Jeff Sharkey Committed by Bryan Ferris
Browse files

RESTRICT AUTOMERGE Strict SQLiteQueryBuilder needs to be stricter.

Malicious callers can leak side-channel information by using
subqueries in any untrusted inputs where SQLite allows "expr" values.

This change offers setStrictGrammar() to prevent this by outright
blocking subqueries in WHERE and HAVING clauses, and by requiring
that GROUP BY and ORDER BY clauses be composed only of valid columns.

This change also offers setStrictColumns() to require that all
untrusted column names are valid, such as those in ContentValues.

Relaxes to always allow aggregation operators on returned columns,
since untrusted callers can always calculate these manually.

Bug: 135270103, 135269143
Test: atest android.database.sqlite.cts.SQLiteQueryBuilderTest
Test: atest FrameworksCoreTests:android.database.sqlite.SQLiteTokenizerTest
Change-Id: I0dacb53170ce573a2fe103cbff455782bfdb5d41
parent 13f49c42
Loading
Loading
Loading
Loading
+327 −78
Original line number Diff line number Diff line
@@ -30,11 +30,14 @@ import android.text.TextUtils;
import android.util.ArrayMap;
import android.util.Log;

import com.android.internal.util.ArrayUtils;

import libcore.util.EmptyArray;

import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
@@ -49,14 +52,11 @@ import java.util.regex.Pattern;
public class SQLiteQueryBuilder {
    private static final String TAG = "SQLiteQueryBuilder";

    private static final Pattern sLimitPattern =
            Pattern.compile("\\s*\\d+\\s*(,\\s*\\d+\\s*)?");
    private static final Pattern sAggregationPattern = Pattern.compile(
            "(?i)(AVG|COUNT|MAX|MIN|SUM|TOTAL|GROUP_CONCAT)\\((.+)\\)");

    private Map<String, String> mProjectionMap = null;
    private List<Pattern> mProjectionGreylist = null;
    private boolean mProjectionAggregationAllowed = false;

    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
    private String mTables = "";
@@ -65,7 +65,12 @@ public class SQLiteQueryBuilder {
    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
    private boolean mDistinct;
    private SQLiteDatabase.CursorFactory mFactory;
    private boolean mStrict;

    private static final int STRICT_PARENTHESES = 1 << 0;
    private static final int STRICT_COLUMNS = 1 << 1;
    private static final int STRICT_GRAMMAR = 1 << 2;

    private int mStrictFlags;

    public SQLiteQueryBuilder() {
        mDistinct = false;
@@ -208,14 +213,23 @@ public class SQLiteQueryBuilder {
        return mProjectionGreylist;
    }

    /** {@hide} */
    /**
     * @deprecated Projection aggregation is now always allowed
     *
     * @hide
     */
    @Deprecated
    public void setProjectionAggregationAllowed(boolean projectionAggregationAllowed) {
        mProjectionAggregationAllowed = projectionAggregationAllowed;
    }

    /** {@hide} */
    /**
     * @deprecated Projection aggregation is now always allowed
     *
     * @hide
     */
    @Deprecated
    public boolean isProjectionAggregationAllowed() {
        return mProjectionAggregationAllowed;
        return true;
    }

    /**
@@ -258,8 +272,12 @@ public class SQLiteQueryBuilder {
     * </ul>
     * By default, this value is false.
     */
    public void setStrict(boolean flag) {
        mStrict = flag;
    public void setStrict(boolean strict) {
        if (strict) {
            mStrictFlags |= STRICT_PARENTHESES;
        } else {
            mStrictFlags &= ~STRICT_PARENTHESES;
        }
    }

    /**
@@ -267,7 +285,75 @@ public class SQLiteQueryBuilder {
     * {@link #setStrict(boolean)}.
     */
    public boolean isStrict() {
        return mStrict;
        return (mStrictFlags & STRICT_PARENTHESES) != 0;
    }

    /**
     * When enabled, verify that all projections and {@link ContentValues} only
     * contain valid columns as defined by {@link #setProjectionMap(Map)}.
     * <p>
     * This enforcement applies to {@link #insert}, {@link #query}, and
     * {@link #update} operations. Any enforcement failures will throw an
     * {@link IllegalArgumentException}.
     *
     * {@hide}
     */
    public void setStrictColumns(boolean strictColumns) {
        if (strictColumns) {
            mStrictFlags |= STRICT_COLUMNS;
        } else {
            mStrictFlags &= ~STRICT_COLUMNS;
        }
    }

    /**
     * Get if the query is marked as strict, as last configured by
     * {@link #setStrictColumns(boolean)}.
     *
     * {@hide}
     */
    public boolean isStrictColumns() {
        return (mStrictFlags & STRICT_COLUMNS) != 0;
    }

    /**
     * When enabled, verify that all untrusted SQL conforms to a restricted SQL
     * grammar. Here are the restrictions applied:
     * <ul>
     * <li>In {@code WHERE} and {@code HAVING} clauses: subqueries, raising, and
     * windowing terms are rejected.
     * <li>In {@code GROUP BY} clauses: only valid columns are allowed.
     * <li>In {@code ORDER BY} clauses: only valid columns, collation, and
     * ordering terms are allowed.
     * <li>In {@code LIMIT} clauses: only numerical values and offset terms are
     * allowed.
     * </ul>
     * All column references must be valid as defined by
     * {@link #setProjectionMap(Map)}.
     * <p>
     * This enforcement applies to {@link #query}, {@link #update} and
     * {@link #delete} operations. This enforcement does not apply to trusted
     * inputs, such as those provided by {@link #appendWhere}. Any enforcement
     * failures will throw an {@link IllegalArgumentException}.
     *
     * {@hide}
     */
    public void setStrictGrammar(boolean strictGrammar) {
        if (strictGrammar) {
            mStrictFlags |= STRICT_GRAMMAR;
        } else {
            mStrictFlags &= ~STRICT_GRAMMAR;
        }
    }

    /**
     * Get if the query is marked as strict, as last configured by
     * {@link #setStrictGrammar(boolean)}.
     *
     * {@hide}
     */
    public boolean isStrictGrammar() {
        return (mStrictFlags & STRICT_GRAMMAR) != 0;
    }

    /**
@@ -303,9 +389,6 @@ public class SQLiteQueryBuilder {
            throw new IllegalArgumentException(
                    "HAVING clauses are only permitted when using a groupBy clause");
        }
        if (!TextUtils.isEmpty(limit) && !sLimitPattern.matcher(limit).matches()) {
            throw new IllegalArgumentException("invalid LIMIT clauses:" + limit);
        }

        StringBuilder query = new StringBuilder(120);

@@ -479,7 +562,13 @@ public class SQLiteQueryBuilder {
                projectionIn, selection, groupBy, having,
                sortOrder, limit);

        if (mStrict && selection != null && selection.length() > 0) {
        if (isStrictColumns()) {
            enforceStrictColumns(projectionIn);
        }
        if (isStrictGrammar()) {
            enforceStrictGrammar(selection, groupBy, having, sortOrder, limit);
        }
        if (isStrict()) {
            // Validate the user-supplied selection to detect syntactic anomalies
            // in the selection string that could indicate a SQL injection attempt.
            // The idea is to ensure that the selection clause is a valid SQL expression
@@ -497,7 +586,7 @@ public class SQLiteQueryBuilder {

            // Execute wrapped query for extra protection
            final String wrappedSql = buildQuery(projectionIn, wrap(selection), groupBy,
                    having, sortOrder, limit);
                    wrap(having), sortOrder, limit);
            sql = wrappedSql;
        } else {
            // Execute unwrapped query
@@ -518,6 +607,42 @@ public class SQLiteQueryBuilder {
                cancellationSignal); // will throw if query is invalid
    }

    /**
     * Perform an insert by combining all current settings and the
     * information passed into this method.
     *
     * @param db the database to insert on
     * @return the row ID of the newly inserted row, or -1 if an error occurred
     *
     * {@hide}
     */
    public long insert(@NonNull SQLiteDatabase db, @NonNull ContentValues values) {
        Objects.requireNonNull(mTables, "No tables defined");
        Objects.requireNonNull(db, "No database defined");
        Objects.requireNonNull(values, "No values defined");

        if (isStrictColumns()) {
            enforceStrictColumns(values);
        }

        final String sql = buildInsert(values);

        final ArrayMap<String, Object> rawValues = values.getValues();
        final int valuesLength = rawValues.size();
        final Object[] sqlArgs = new Object[valuesLength];
        for (int i = 0; i < sqlArgs.length; i++) {
            sqlArgs[i] = rawValues.valueAt(i);
        }
        if (Log.isLoggable(TAG, Log.DEBUG)) {
            if (Build.IS_DEBUGGABLE) {
                Log.d(TAG, sql + " with args " + Arrays.toString(sqlArgs));
            } else {
                Log.d(TAG, sql);
            }
        }
        return db.executeSql(sql, sqlArgs);
    }

    /**
     * Perform an update by combining all current settings and the
     * information passed into this method.
@@ -541,7 +666,13 @@ public class SQLiteQueryBuilder {
        final String sql;
        final String unwrappedSql = buildUpdate(values, selection);

        if (mStrict) {
        if (isStrictColumns()) {
            enforceStrictColumns(values);
        }
        if (isStrictGrammar()) {
            enforceStrictGrammar(selection, null, null, null, null);
        }
        if (isStrict()) {
            // Validate the user-supplied selection to detect syntactic anomalies
            // in the selection string that could indicate a SQL injection attempt.
            // The idea is to ensure that the selection clause is a valid SQL expression
@@ -610,7 +741,10 @@ public class SQLiteQueryBuilder {
        final String sql;
        final String unwrappedSql = buildDelete(selection);

        if (mStrict) {
        if (isStrictGrammar()) {
            enforceStrictGrammar(selection, null, null, null, null);
        }
        if (isStrict()) {
            // Validate the user-supplied selection to detect syntactic anomalies
            // in the selection string that could indicate a SQL injection attempt.
            // The idea is to ensure that the selection clause is a valid SQL expression
@@ -645,6 +779,81 @@ public class SQLiteQueryBuilder {
        return db.executeSql(sql, sqlArgs);
    }

    private void enforceStrictColumns(@Nullable String[] projection) {
        Objects.requireNonNull(mProjectionMap, "No projection map defined");

        computeProjection(projection);
    }

    private void enforceStrictColumns(@NonNull ContentValues values) {
        Objects.requireNonNull(mProjectionMap, "No projection map defined");

        final ArrayMap<String, Object> rawValues = values.getValues();
        for (int i = 0; i < rawValues.size(); i++) {
            final String column = rawValues.keyAt(i);
            if (!mProjectionMap.containsKey(column)) {
                throw new IllegalArgumentException("Invalid column " + column);
            }
        }
    }

    private void enforceStrictGrammar(@Nullable String selection, @Nullable String groupBy,
            @Nullable String having, @Nullable String sortOrder, @Nullable String limit) {
        SQLiteTokenizer.tokenize(selection, SQLiteTokenizer.OPTION_NONE,
                this::enforceStrictGrammarWhereHaving);
        SQLiteTokenizer.tokenize(groupBy, SQLiteTokenizer.OPTION_NONE,
                this::enforceStrictGrammarGroupBy);
        SQLiteTokenizer.tokenize(having, SQLiteTokenizer.OPTION_NONE,
                this::enforceStrictGrammarWhereHaving);
        SQLiteTokenizer.tokenize(sortOrder, SQLiteTokenizer.OPTION_NONE,
                this::enforceStrictGrammarOrderBy);
        SQLiteTokenizer.tokenize(limit, SQLiteTokenizer.OPTION_NONE,
                this::enforceStrictGrammarLimit);
    }

    private void enforceStrictGrammarWhereHaving(@NonNull String token) {
        if (isTableOrColumn(token)) return;
        if (SQLiteTokenizer.isFunction(token)) return;
        if (SQLiteTokenizer.isType(token)) return;

        // NOTE: we explicitly don't allow SELECT subqueries, since they could
        // leak data that should have been filtered by the trusted where clause
        switch (token.toUpperCase(Locale.US)) {
            case "AND": case "AS": case "BETWEEN": case "BINARY":
            case "CASE": case "CAST": case "COLLATE": case "DISTINCT":
            case "ELSE": case "END": case "ESCAPE": case "EXISTS":
            case "GLOB": case "IN": case "IS": case "ISNULL":
            case "LIKE": case "MATCH": case "NOCASE": case "NOT":
            case "NOTNULL": case "NULL": case "OR": case "REGEXP":
            case "RTRIM": case "THEN": case "WHEN":
                return;
        }
        throw new IllegalArgumentException("Invalid token " + token);
    }

    private void enforceStrictGrammarGroupBy(@NonNull String token) {
        if (isTableOrColumn(token)) return;
        throw new IllegalArgumentException("Invalid token " + token);
    }

    private void enforceStrictGrammarOrderBy(@NonNull String token) {
        if (isTableOrColumn(token)) return;
        switch (token.toUpperCase(Locale.US)) {
            case "COLLATE": case "ASC": case "DESC":
            case "BINARY": case "RTRIM": case "NOCASE":
                return;
        }
        throw new IllegalArgumentException("Invalid token " + token);
    }

    private void enforceStrictGrammarLimit(@NonNull String token) {
        switch (token.toUpperCase(Locale.US)) {
            case "OFFSET":
                return;
        }
        throw new IllegalArgumentException("Invalid token " + token);
    }

    /**
     * Construct a {@code SELECT} statement suitable for use in a group of
     * {@code SELECT} statements that will be joined through {@code UNION} operators
@@ -697,6 +906,35 @@ public class SQLiteQueryBuilder {
        return buildQuery(projectionIn, selection, groupBy, having, sortOrder, limit);
    }

    /** {@hide} */
    public String buildInsert(ContentValues values) {
        if (values == null || values.isEmpty()) {
            throw new IllegalArgumentException("Empty values");
        }

        StringBuilder sql = new StringBuilder(120);
        sql.append("INSERT INTO ");
        sql.append(SQLiteDatabase.findEditTable(mTables));
        sql.append(" (");

        final ArrayMap<String, Object> rawValues = values.getValues();
        for (int i = 0; i < rawValues.size(); i++) {
            if (i > 0) {
                sql.append(',');
            }
            sql.append(rawValues.keyAt(i));
        }
        sql.append(") VALUES (");
        for (int i = 0; i < rawValues.size(); i++) {
            if (i > 0) {
                sql.append(',');
            }
            sql.append('?');
        }
        sql.append(")");
        return sql.toString();
    }

    /** {@hide} */
    public String buildUpdate(ContentValues values, String selection) {
        if (values == null || values.isEmpty()) {
@@ -705,7 +943,7 @@ public class SQLiteQueryBuilder {

        StringBuilder sql = new StringBuilder(120);
        sql.append("UPDATE ");
        sql.append(mTables);
        sql.append(SQLiteDatabase.findEditTable(mTables));
        sql.append(" SET ");

        final ArrayMap<String, Object> rawValues = values.getValues();
@@ -726,7 +964,7 @@ public class SQLiteQueryBuilder {
    public String buildDelete(String selection) {
        StringBuilder sql = new StringBuilder(120);
        sql.append("DELETE FROM ");
        sql.append(mTables);
        sql.append(SQLiteDatabase.findEditTable(mTables));

        final String where = computeWhere(selection);
        appendClause(sql, " WHERE ", where);
@@ -868,20 +1106,54 @@ public class SQLiteQueryBuilder {

    /** {@hide} */
    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
    public String[] computeProjection(String[] projectionIn) {
        if (projectionIn != null && projectionIn.length > 0) {
            if (mProjectionMap != null) {
                String[] projection = new String[projectionIn.length];
                int length = projectionIn.length;
    public @Nullable String[] computeProjection(@Nullable String[] projectionIn) {
        if (!ArrayUtils.isEmpty(projectionIn)) {
            String[] projectionOut = new String[projectionIn.length];
            for (int i = 0; i < projectionIn.length; i++) {
                projectionOut[i] = computeSingleProjectionOrThrow(projectionIn[i]);
            }
            return projectionOut;
        } else if (mProjectionMap != null) {
            // Return all columns in projection map.
            Set<Entry<String, String>> entrySet = mProjectionMap.entrySet();
            String[] projection = new String[entrySet.size()];
            Iterator<Entry<String, String>> entryIter = entrySet.iterator();
            int i = 0;

            while (entryIter.hasNext()) {
                Entry<String, String> entry = entryIter.next();

                // Don't include the _count column when people ask for no projection.
                if (entry.getKey().equals(BaseColumns._COUNT)) {
                    continue;
                }
                projection[i++] = entry.getValue();
            }
            return projection;
        }
        return null;
    }

    private @NonNull String computeSingleProjectionOrThrow(@NonNull String userColumn) {
        final String column = computeSingleProjection(userColumn);
        if (column != null) {
            return column;
        } else {
            throw new IllegalArgumentException("Invalid column " + userColumn);
        }
    }

    private @Nullable String computeSingleProjection(@NonNull String userColumn) {
        // When no mapping provided, anything goes
        if (mProjectionMap == null) {
            return userColumn;
        }

                for (int i = 0; i < length; i++) {
        String operator = null;
                    String userColumn = projectionIn[i];
        String column = mProjectionMap.get(userColumn);

                    // If aggregation is allowed, extract the underlying column
                    // that may be aggregated
                    if (mProjectionAggregationAllowed) {
        // When no direct match found, look for aggregation
        if (column == null) {
            final Matcher matcher = sAggregationPattern.matcher(userColumn);
            if (matcher.matches()) {
                operator = matcher.group(1);
@@ -891,15 +1163,13 @@ public class SQLiteQueryBuilder {
        }

        if (column != null) {
                        projection[i] = maybeWithOperator(operator, column);
                        continue;
            return maybeWithOperator(operator, column);
        }

                    if (!mStrict &&
                            ( userColumn.contains(" AS ") || userColumn.contains(" as "))) {
        if (mStrictFlags == 0
                && (userColumn.contains(" AS ") || userColumn.contains(" as "))) {
            /* A column alias already exist */
                        projection[i] = maybeWithOperator(operator, userColumn);
                        continue;
            return maybeWithOperator(operator, userColumn);
        }

        // If greylist is configured, we might be willing to let
@@ -915,37 +1185,16 @@ public class SQLiteQueryBuilder {

            if (match) {
                Log.w(TAG, "Allowing abusive custom column: " + userColumn);
                            projection[i] = maybeWithOperator(operator, userColumn);
                            continue;
                return maybeWithOperator(operator, userColumn);
            }
        }

                    throw new IllegalArgumentException("Invalid column "
                            + projectionIn[i]);
                }
                return projection;
            } else {
                return projectionIn;
        return null;
    }
        } else if (mProjectionMap != null) {
            // Return all columns in projection map.
            Set<Entry<String, String>> entrySet = mProjectionMap.entrySet();
            String[] projection = new String[entrySet.size()];
            Iterator<Entry<String, String>> entryIter = entrySet.iterator();
            int i = 0;

            while (entryIter.hasNext()) {
                Entry<String, String> entry = entryIter.next();

                // Don't include the _count column when people ask for no projection.
                if (entry.getKey().equals(BaseColumns._COUNT)) {
                    continue;
                }
                projection[i++] = entry.getValue();
            }
            return projection;
        }
        return null;
    private boolean isTableOrColumn(String token) {
        if (mTables.equals(token)) return true;
        return computeSingleProjection(token) != null;
    }

    /** {@hide} */
+297 −0

File added.

Preview size limit exceeded, changes collapsed.

+173 −0

File added.

Preview size limit exceeded, changes collapsed.