From ae647ba84ff20e507c12bc7d72a2ac63197958b6 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Mon, 9 Jul 2018 12:16:26 -0600 Subject: Remove "public" download feature. It was never a supported API, and has been reported as causing security issues, so remove it. Bug: 111084083 Test: builds Change-Id: I26345b192ffd55216bb8c8fdb82cb5869d68d3db (cherry picked from commit ce9f204ac493f000cd3020e195fd5038d0cec1e2) --- src/com/android/providers/downloads/DownloadProvider.java | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) (limited to 'src/com/android/providers/downloads') diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java index e177da17..db7db65a 100644 --- a/src/com/android/providers/downloads/DownloadProvider.java +++ b/src/com/android/providers/downloads/DownloadProvider.java @@ -105,11 +105,6 @@ public final class DownloadProvider extends ContentProvider { private static final int ALL_DOWNLOADS_ID = 4; /** URI matcher constant for the URI of a download's request headers */ private static final int REQUEST_HEADERS_URI = 5; - /** URI matcher constant for the public URI returned by - * {@link DownloadManager#getUriForDownloadedFile(long)} if the given downloaded file - * is publicly accessible. - */ - private static final int PUBLIC_DOWNLOAD_ID = 6; static { sURIMatcher.addURI("downloads", "my_downloads", MY_DOWNLOADS); sURIMatcher.addURI("downloads", "my_downloads/#", MY_DOWNLOADS_ID); @@ -127,9 +122,6 @@ public final class DownloadProvider extends ContentProvider { sURIMatcher.addURI("downloads", "download/#/" + Downloads.Impl.RequestHeaders.URI_SEGMENT, REQUEST_HEADERS_URI); - sURIMatcher.addURI("downloads", - Downloads.Impl.PUBLICLY_ACCESSIBLE_DOWNLOADS_URI_SEGMENT + "/#", - PUBLIC_DOWNLOAD_ID); } /** Different base URIs that could be used to access an individual download */ @@ -526,8 +518,7 @@ public final class DownloadProvider extends ContentProvider { return DOWNLOAD_LIST_TYPE; } case MY_DOWNLOADS_ID: - case ALL_DOWNLOADS_ID: - case PUBLIC_DOWNLOAD_ID: { + case ALL_DOWNLOADS_ID: { // return the mimetype of this id from the database final String id = getDownloadIdFromUri(uri); final SQLiteDatabase db = mOpenHelper.getReadableDatabase(); @@ -1234,8 +1225,7 @@ public final class DownloadProvider extends ContentProvider { int uriMatch) { SqlSelection selection = new SqlSelection(); selection.appendClause(where, whereArgs); - if (uriMatch == MY_DOWNLOADS_ID || uriMatch == ALL_DOWNLOADS_ID || - uriMatch == PUBLIC_DOWNLOAD_ID) { + if (uriMatch == MY_DOWNLOADS_ID || uriMatch == ALL_DOWNLOADS_ID) { selection.appendClause(Downloads.Impl._ID + " = ?", getDownloadIdFromUri(uri)); } if ((uriMatch == MY_DOWNLOADS || uriMatch == MY_DOWNLOADS_ID) -- cgit v1.2.3 From 4f9e4016c1e96ebe8c10360e32d0bf3846b97c57 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Mon, 16 Jul 2018 13:18:04 -0600 Subject: DO NOT MERGE. All untrusted selections must go through builder. When accepting untrusted selections, they must be passed directly to SQLiteQueryBuilder to ensure that setStrict() can be applied to check for malicious callers sending unbalanced parentheses. This means we can't mix local and remote selections; they always need to be kept separate. Use newly added SQLiteQueryBuilder functionality to apply strict detection to update() and delete() calls. Only allow the owner of a particular download to query the headers for that download. Only delete headers for a download once we've confirmed that caller can modify that download. Test: atest packages/providers/DownloadProvider/tests/ Test: atest cts/tests/app/src/android/app/cts/DownloadManagerTest.java Bug: 111085900 Change-Id: I9fd8e0d3cf80d7603bf0092f36fe449467090821 Merged-In: I9fd8e0d3cf80d7603bf0092f36fe449467090821 (cherry picked from commit 64b55ea82b1f394369237601ae1f1c78b776aabc) --- .../providers/downloads/DownloadProvider.java | 196 ++++++++++----------- 1 file changed, 91 insertions(+), 105 deletions(-) (limited to 'src/com/android/providers/downloads') diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java index db7db65a..f8d5aae2 100644 --- a/src/com/android/providers/downloads/DownloadProvider.java +++ b/src/com/android/providers/downloads/DownloadProvider.java @@ -20,7 +20,9 @@ import static android.provider.BaseColumns._ID; import static android.provider.Downloads.Impl.COLUMN_DESTINATION; import static android.provider.Downloads.Impl.COLUMN_MEDIA_SCANNED; import static android.provider.Downloads.Impl.COLUMN_MIME_TYPE; +import static android.provider.Downloads.Impl.COLUMN_OTHER_UID; import static android.provider.Downloads.Impl.DESTINATION_NON_DOWNLOADMANAGER_DOWNLOAD; +import static android.provider.Downloads.Impl.PERMISSION_ACCESS_ALL; import static android.provider.Downloads.Impl._DATA; import android.app.AppOpsManager; @@ -99,12 +101,14 @@ public final class DownloadProvider extends ContentProvider { private static final int MY_DOWNLOADS = 1; /** URI matcher constant for the URI of an individual download belonging to the calling UID */ private static final int MY_DOWNLOADS_ID = 2; + /** URI matcher constant for the URI of a download's request headers */ + private static final int MY_DOWNLOADS_ID_HEADERS = 3; /** URI matcher constant for the URI of all downloads in the system */ - private static final int ALL_DOWNLOADS = 3; + private static final int ALL_DOWNLOADS = 4; /** URI matcher constant for the URI of an individual download */ - private static final int ALL_DOWNLOADS_ID = 4; + private static final int ALL_DOWNLOADS_ID = 5; /** URI matcher constant for the URI of a download's request headers */ - private static final int REQUEST_HEADERS_URI = 5; + private static final int ALL_DOWNLOADS_ID_HEADERS = 6; static { sURIMatcher.addURI("downloads", "my_downloads", MY_DOWNLOADS); sURIMatcher.addURI("downloads", "my_downloads/#", MY_DOWNLOADS_ID); @@ -112,16 +116,16 @@ public final class DownloadProvider extends ContentProvider { sURIMatcher.addURI("downloads", "all_downloads/#", ALL_DOWNLOADS_ID); sURIMatcher.addURI("downloads", "my_downloads/#/" + Downloads.Impl.RequestHeaders.URI_SEGMENT, - REQUEST_HEADERS_URI); + MY_DOWNLOADS_ID_HEADERS); sURIMatcher.addURI("downloads", "all_downloads/#/" + Downloads.Impl.RequestHeaders.URI_SEGMENT, - REQUEST_HEADERS_URI); + ALL_DOWNLOADS_ID_HEADERS); // temporary, for backwards compatibility sURIMatcher.addURI("downloads", "download", MY_DOWNLOADS); sURIMatcher.addURI("downloads", "download/#", MY_DOWNLOADS_ID); sURIMatcher.addURI("downloads", "download/#/" + Downloads.Impl.RequestHeaders.URI_SEGMENT, - REQUEST_HEADERS_URI); + MY_DOWNLOADS_ID_HEADERS); } /** Different base URIs that could be used to access an individual download */ @@ -183,43 +187,6 @@ public final class DownloadProvider extends ContentProvider { private int mSystemUid = -1; private int mDefContainerUid = -1; - /** - * This class encapsulates a SQL where clause and its parameters. It makes it possible for - * shared methods (like {@link DownloadProvider#getWhereClause(Uri, String, String[], int)}) - * to return both pieces of information, and provides some utility logic to ease piece-by-piece - * construction of selections. - */ - private static class SqlSelection { - public StringBuilder mWhereClause = new StringBuilder(); - public List mParameters = new ArrayList(); - - public void appendClause(String newClause, final T... parameters) { - if (newClause == null || newClause.isEmpty()) { - return; - } - if (mWhereClause.length() != 0) { - mWhereClause.append(" AND "); - } - mWhereClause.append("("); - mWhereClause.append(newClause); - mWhereClause.append(")"); - if (parameters != null) { - for (Object parameter : parameters) { - mParameters.add(parameter.toString()); - } - } - } - - public String getSelection() { - return mWhereClause.toString(); - } - - public String[] getParameters() { - String[] array = new String[mParameters.size()]; - return mParameters.toArray(array); - } - } - /** * Creates and updated database on demand when opening it. * Helper class to create database the first time the provider is @@ -933,15 +900,23 @@ public final class DownloadProvider extends ContentProvider { throw new IllegalArgumentException("Unknown URI: " + uri); } - if (match == REQUEST_HEADERS_URI) { + if (match == MY_DOWNLOADS_ID_HEADERS || match == ALL_DOWNLOADS_ID_HEADERS) { if (projection != null || selection != null || sort != null) { throw new UnsupportedOperationException("Request header queries do not support " + "projections, selections or sorting"); } - return queryRequestHeaders(db, uri); - } - SqlSelection fullSelection = getWhereClause(uri, selection, selectionArgs, match); + // Headers are only available to callers with full access. + getContext().enforceCallingOrSelfPermission( + Downloads.Impl.PERMISSION_ACCESS_ALL, Constants.TAG); + + final SQLiteQueryBuilder qb = getQueryBuilder(uri, match); + projection = new String[] { + Downloads.Impl.RequestHeaders.COLUMN_HEADER, + Downloads.Impl.RequestHeaders.COLUMN_VALUE + }; + return qb.query(db, projection, null, null, null, null, null); + } if (shouldRestrictVisibility()) { if (projection == null) { @@ -969,11 +944,8 @@ public final class DownloadProvider extends ContentProvider { logVerboseQueryInfo(projection, selection, selectionArgs, sort, db); } - SQLiteQueryBuilder builder = new SQLiteQueryBuilder(); - builder.setTables(DB_TABLE); - builder.setStrict(true); - Cursor ret = builder.query(db, projection, fullSelection.getSelection(), - fullSelection.getParameters(), null, null, sort); + final SQLiteQueryBuilder qb = getQueryBuilder(uri, match); + final Cursor ret = qb.query(db, projection, selection, selectionArgs, null, null, sort); if (ret != null) { ret.setNotificationUri(getContext().getContentResolver(), uri); @@ -1058,35 +1030,6 @@ public final class DownloadProvider extends ContentProvider { } } - /** - * Handle a query for the custom request headers registered for a download. - */ - private Cursor queryRequestHeaders(SQLiteDatabase db, Uri uri) { - String where = Downloads.Impl.RequestHeaders.COLUMN_DOWNLOAD_ID + "=" - + getDownloadIdFromUri(uri); - String[] projection = new String[] {Downloads.Impl.RequestHeaders.COLUMN_HEADER, - Downloads.Impl.RequestHeaders.COLUMN_VALUE}; - return db.query(Downloads.Impl.RequestHeaders.HEADERS_DB_TABLE, projection, where, - null, null, null, null); - } - - /** - * Delete request headers for downloads matching the given query. - */ - private void deleteRequestHeaders(SQLiteDatabase db, String where, String[] whereArgs) { - String[] projection = new String[] {Downloads.Impl._ID}; - Cursor cursor = db.query(DB_TABLE, projection, where, whereArgs, null, null, null, null); - try { - for (cursor.moveToFirst(); !cursor.isAfterLast(); cursor.moveToNext()) { - long id = cursor.getLong(0); - String idWhere = Downloads.Impl.RequestHeaders.COLUMN_DOWNLOAD_ID + "=" + id; - db.delete(Downloads.Impl.RequestHeaders.HEADERS_DB_TABLE, idWhere, null); - } - } finally { - cursor.close(); - } - } - /** * @return true if we should restrict the columns readable by this caller */ @@ -1169,13 +1112,11 @@ public final class DownloadProvider extends ContentProvider { break; } - final SqlSelection selection = getWhereClause(uri, where, whereArgs, match); - count = db.update(DB_TABLE, filteredValues, selection.getSelection(), - selection.getParameters()); + final SQLiteQueryBuilder qb = getQueryBuilder(uri, match); + count = qb.update(db, filteredValues, where, whereArgs); if (updateSchedule || isCompleting) { final long token = Binder.clearCallingIdentity(); - try (Cursor cursor = db.query(DB_TABLE, null, selection.getSelection(), - selection.getParameters(), null, null, null)) { + try (Cursor cursor = qb.query(db, null, where, whereArgs, null, null, null)) { final DownloadInfo.Reader reader = new DownloadInfo.Reader(resolver, cursor); final DownloadInfo info = new DownloadInfo(context); @@ -1221,21 +1162,64 @@ public final class DownloadProvider extends ContentProvider { } } - private SqlSelection getWhereClause(final Uri uri, final String where, final String[] whereArgs, - int uriMatch) { - SqlSelection selection = new SqlSelection(); - selection.appendClause(where, whereArgs); - if (uriMatch == MY_DOWNLOADS_ID || uriMatch == ALL_DOWNLOADS_ID) { - selection.appendClause(Downloads.Impl._ID + " = ?", getDownloadIdFromUri(uri)); + /** + * Create a query builder that filters access to the underlying database + * based on both the requested {@link Uri} and permissions of the caller. + */ + private SQLiteQueryBuilder getQueryBuilder(final Uri uri, int match) { + final String table; + final StringBuilder where = new StringBuilder(); + switch (match) { + // The "my_downloads" view normally limits the caller to operating + // on downloads that they either directly own, or have been given + // indirect ownership of via OTHER_UID. + case MY_DOWNLOADS_ID: + appendWhereExpression(where, _ID + "=" + getDownloadIdFromUri(uri)); + // fall-through + case MY_DOWNLOADS: + table = DB_TABLE; + if (getContext().checkCallingOrSelfPermission( + PERMISSION_ACCESS_ALL) != PackageManager.PERMISSION_GRANTED) { + appendWhereExpression(where, Constants.UID + "=" + Binder.getCallingUid() + + " OR " + COLUMN_OTHER_UID + "=" + Binder.getCallingUid()); + } + break; + + // The "all_downloads" view is already limited via + // to only callers holding the ACCESS_ALL_DOWNLOADS permission, but + // access may also be delegated via Uri permission grants. + case ALL_DOWNLOADS_ID: + appendWhereExpression(where, _ID + "=" + getDownloadIdFromUri(uri)); + // fall-through + case ALL_DOWNLOADS: + table = DB_TABLE; + break; + + // Headers are limited to callers holding the ACCESS_ALL_DOWNLOADS + // permission, since they're only needed for executing downloads. + case MY_DOWNLOADS_ID_HEADERS: + case ALL_DOWNLOADS_ID_HEADERS: + table = Downloads.Impl.RequestHeaders.HEADERS_DB_TABLE; + appendWhereExpression(where, Downloads.Impl.RequestHeaders.COLUMN_DOWNLOAD_ID + "=" + + getDownloadIdFromUri(uri)); + break; + + default: + throw new UnsupportedOperationException("Unknown URI: " + uri); } - if ((uriMatch == MY_DOWNLOADS || uriMatch == MY_DOWNLOADS_ID) - && getContext().checkCallingOrSelfPermission(Downloads.Impl.PERMISSION_ACCESS_ALL) - != PackageManager.PERMISSION_GRANTED) { - selection.appendClause( - Constants.UID + "= ? OR " + Downloads.Impl.COLUMN_OTHER_UID + "= ?", - Binder.getCallingUid(), Binder.getCallingUid()); + + final SQLiteQueryBuilder qb = new SQLiteQueryBuilder(); + qb.setStrict(true); + qb.setTables(table); + qb.appendWhere(where); + return qb; + } + + private static void appendWhereExpression(StringBuilder sb, String expression) { + if (sb.length() > 0) { + sb.append(" AND "); } - return selection; + sb.append('(').append(expression).append(')'); } /** @@ -1259,11 +1243,8 @@ public final class DownloadProvider extends ContentProvider { case MY_DOWNLOADS_ID: case ALL_DOWNLOADS: case ALL_DOWNLOADS_ID: - final SqlSelection selection = getWhereClause(uri, where, whereArgs, match); - deleteRequestHeaders(db, selection.getSelection(), selection.getParameters()); - - try (Cursor cursor = db.query(DB_TABLE, null, selection.getSelection(), - selection.getParameters(), null, null, null)) { + final SQLiteQueryBuilder qb = getQueryBuilder(uri, match); + try (Cursor cursor = qb.query(db, null, where, whereArgs, null, null, null)) { final DownloadInfo.Reader reader = new DownloadInfo.Reader(resolver, cursor); final DownloadInfo info = new DownloadInfo(context); while (cursor.moveToNext()) { @@ -1305,10 +1286,15 @@ public final class DownloadProvider extends ContentProvider { if (!Downloads.Impl.isStatusCompleted(info.mStatus)) { info.sendIntentIfRequested(); } + + // Delete any headers for this download + db.delete(Downloads.Impl.RequestHeaders.HEADERS_DB_TABLE, + Downloads.Impl.RequestHeaders.COLUMN_DOWNLOAD_ID + "=?", + new String[] { Long.toString(info.mId) }); } } - count = db.delete(DB_TABLE, selection.getSelection(), selection.getParameters()); + count = qb.delete(db, where, whereArgs); break; default: -- cgit v1.2.3 From 54519209a627e515763406f34fc3ae232b766cd0 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 17 Jul 2019 18:54:49 -0600 Subject: RESTRICT AUTOMERGE Enable stricter SQLiteQueryBuilder options. Malicious callers can leak side-channel information by using subqueries in any untrusted inputs where SQLite allows "expr" values. This change starts using setStrictColumns() and setStrictGrammar() on SQLiteQueryBuilder to block this class of attacks. This means we now need to define the projection mapping of valid columns, which consists of both the columns defined in the public API and columns read internally by DownloadInfo.Reader. We're okay growing sAppReadableColumnsSet like this, since we're relying on our trusted WHERE clause to filter away any rows that don't belong to the calling UID. Remove the legacy Lexer code, since we're now internally relying on the robust and well-tested SQLiteTokenizer logic. Bug: 135270103 Bug: 135269143 Test: atest DownloadProviderTests Test: atest CtsAppTestCases:android.app.cts.DownloadManagerTest Change-Id: I302091ceda3591785b2124575e89dad19bc97469 (cherry picked from commit d3e5c766a143853580dd6642a4a32c5d1a6f9fb1) --- .../providers/downloads/DownloadProvider.java | 189 ++++++++------- src/com/android/providers/downloads/Helpers.java | 263 +-------------------- 2 files changed, 107 insertions(+), 345 deletions(-) (limited to 'src/com/android/providers/downloads') diff --git a/src/com/android/providers/downloads/DownloadProvider.java b/src/com/android/providers/downloads/DownloadProvider.java index f8d5aae2..78ab7583 100644 --- a/src/com/android/providers/downloads/DownloadProvider.java +++ b/src/com/android/providers/downloads/DownloadProvider.java @@ -55,13 +55,13 @@ import android.provider.Downloads; import android.provider.OpenableColumns; import android.text.TextUtils; import android.text.format.DateUtils; +import android.util.ArrayMap; import android.util.Log; import com.android.internal.util.IndentingPrintWriter; import libcore.io.IoUtils; -import com.google.android.collect.Maps; import com.google.common.annotations.VisibleForTesting; import java.io.File; @@ -70,11 +70,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.PrintWriter; import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.HashSet; import java.util.Iterator; -import java.util.List; import java.util.Map; /** @@ -134,48 +130,107 @@ public final class DownloadProvider extends ContentProvider { Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, }; - private static final String[] sAppReadableColumnsArray = new String[] { - Downloads.Impl._ID, - Downloads.Impl.COLUMN_APP_DATA, - Downloads.Impl._DATA, - Downloads.Impl.COLUMN_MIME_TYPE, - Downloads.Impl.COLUMN_VISIBILITY, - Downloads.Impl.COLUMN_DESTINATION, - Downloads.Impl.COLUMN_CONTROL, - Downloads.Impl.COLUMN_STATUS, - Downloads.Impl.COLUMN_LAST_MODIFICATION, - Downloads.Impl.COLUMN_NOTIFICATION_PACKAGE, - Downloads.Impl.COLUMN_NOTIFICATION_CLASS, - Downloads.Impl.COLUMN_TOTAL_BYTES, - Downloads.Impl.COLUMN_CURRENT_BYTES, - Downloads.Impl.COLUMN_TITLE, - Downloads.Impl.COLUMN_DESCRIPTION, - Downloads.Impl.COLUMN_URI, - Downloads.Impl.COLUMN_IS_VISIBLE_IN_DOWNLOADS_UI, - Downloads.Impl.COLUMN_FILE_NAME_HINT, - Downloads.Impl.COLUMN_MEDIAPROVIDER_URI, - Downloads.Impl.COLUMN_DELETED, - OpenableColumns.DISPLAY_NAME, - OpenableColumns.SIZE, - }; + private static void addMapping(Map map, String column) { + if (!map.containsKey(column)) { + map.put(column, column); + } + } - private static final HashSet sAppReadableColumnsSet; - private static final HashMap sColumnsMap; + private static void addMapping(Map map, String column, String rawColumn) { + if (!map.containsKey(column)) { + map.put(column, rawColumn + " AS " + column); + } + } + private static final Map sDownloadsMap = new ArrayMap<>(); static { - sAppReadableColumnsSet = new HashSet(); - for (int i = 0; i < sAppReadableColumnsArray.length; ++i) { - sAppReadableColumnsSet.add(sAppReadableColumnsArray[i]); - } + final Map map = sDownloadsMap; + + // Columns defined by public API + addMapping(map, DownloadManager.COLUMN_ID, + Downloads.Impl._ID); + addMapping(map, DownloadManager.COLUMN_LOCAL_FILENAME, + Downloads.Impl._DATA); + addMapping(map, DownloadManager.COLUMN_MEDIAPROVIDER_URI); + addMapping(map, DownloadManager.COLUMN_DESTINATION); + addMapping(map, DownloadManager.COLUMN_TITLE); + addMapping(map, DownloadManager.COLUMN_DESCRIPTION); + addMapping(map, DownloadManager.COLUMN_URI); + addMapping(map, DownloadManager.COLUMN_STATUS); + addMapping(map, DownloadManager.COLUMN_FILE_NAME_HINT); + addMapping(map, DownloadManager.COLUMN_MEDIA_TYPE, + Downloads.Impl.COLUMN_MIME_TYPE); + addMapping(map, DownloadManager.COLUMN_TOTAL_SIZE_BYTES, + Downloads.Impl.COLUMN_TOTAL_BYTES); + addMapping(map, DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP, + Downloads.Impl.COLUMN_LAST_MODIFICATION); + addMapping(map, DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR, + Downloads.Impl.COLUMN_CURRENT_BYTES); + addMapping(map, DownloadManager.COLUMN_ALLOW_WRITE); + addMapping(map, DownloadManager.COLUMN_LOCAL_URI, + "'placeholder'"); + addMapping(map, DownloadManager.COLUMN_REASON, + "'placeholder'"); + + // Columns defined by OpenableColumns + addMapping(map, OpenableColumns.DISPLAY_NAME, + Downloads.Impl.COLUMN_TITLE); + addMapping(map, OpenableColumns.SIZE, + Downloads.Impl.COLUMN_TOTAL_BYTES); + + // Allow references to all other columns to support DownloadInfo.Reader; + // we're already using SQLiteQueryBuilder to block access to other rows + // that don't belong to the calling UID. + addMapping(map, Downloads.Impl._ID); + addMapping(map, Downloads.Impl._DATA); + addMapping(map, Downloads.Impl.COLUMN_ALLOWED_NETWORK_TYPES); + addMapping(map, Downloads.Impl.COLUMN_ALLOW_METERED); + addMapping(map, Downloads.Impl.COLUMN_ALLOW_ROAMING); + addMapping(map, Downloads.Impl.COLUMN_ALLOW_WRITE); + addMapping(map, Downloads.Impl.COLUMN_APP_DATA); + addMapping(map, Downloads.Impl.COLUMN_BYPASS_RECOMMENDED_SIZE_LIMIT); + addMapping(map, Downloads.Impl.COLUMN_CONTROL); + addMapping(map, Downloads.Impl.COLUMN_COOKIE_DATA); + addMapping(map, Downloads.Impl.COLUMN_CURRENT_BYTES); + addMapping(map, Downloads.Impl.COLUMN_DELETED); + addMapping(map, Downloads.Impl.COLUMN_DESCRIPTION); + addMapping(map, Downloads.Impl.COLUMN_DESTINATION); + addMapping(map, Downloads.Impl.COLUMN_ERROR_MSG); + addMapping(map, Downloads.Impl.COLUMN_FAILED_CONNECTIONS); + addMapping(map, Downloads.Impl.COLUMN_FILE_NAME_HINT); + addMapping(map, Downloads.Impl.COLUMN_FLAGS); + addMapping(map, Downloads.Impl.COLUMN_IS_PUBLIC_API); + addMapping(map, Downloads.Impl.COLUMN_IS_VISIBLE_IN_DOWNLOADS_UI); + addMapping(map, Downloads.Impl.COLUMN_LAST_MODIFICATION); + addMapping(map, Downloads.Impl.COLUMN_MEDIAPROVIDER_URI); + addMapping(map, Downloads.Impl.COLUMN_MEDIA_SCANNED); + addMapping(map, Downloads.Impl.COLUMN_MIME_TYPE); + addMapping(map, Downloads.Impl.COLUMN_NO_INTEGRITY); + addMapping(map, Downloads.Impl.COLUMN_NOTIFICATION_CLASS); + addMapping(map, Downloads.Impl.COLUMN_NOTIFICATION_EXTRAS); + addMapping(map, Downloads.Impl.COLUMN_NOTIFICATION_PACKAGE); + addMapping(map, Downloads.Impl.COLUMN_OTHER_UID); + addMapping(map, Downloads.Impl.COLUMN_REFERER); + addMapping(map, Downloads.Impl.COLUMN_STATUS); + addMapping(map, Downloads.Impl.COLUMN_TITLE); + addMapping(map, Downloads.Impl.COLUMN_TOTAL_BYTES); + addMapping(map, Downloads.Impl.COLUMN_URI); + addMapping(map, Downloads.Impl.COLUMN_USER_AGENT); + addMapping(map, Downloads.Impl.COLUMN_VISIBILITY); + + addMapping(map, Constants.ETAG); + addMapping(map, Constants.RETRY_AFTER_X_REDIRECT_COUNT); + addMapping(map, Constants.UID); + } - sColumnsMap = Maps.newHashMap(); - sColumnsMap.put(OpenableColumns.DISPLAY_NAME, - Downloads.Impl.COLUMN_TITLE + " AS " + OpenableColumns.DISPLAY_NAME); - sColumnsMap.put(OpenableColumns.SIZE, - Downloads.Impl.COLUMN_TOTAL_BYTES + " AS " + OpenableColumns.SIZE); + private static final Map sHeadersMap = new ArrayMap<>(); + static { + final Map map = sHeadersMap; + addMapping(map, "id"); + addMapping(map, Downloads.Impl.RequestHeaders.COLUMN_DOWNLOAD_ID); + addMapping(map, Downloads.Impl.RequestHeaders.COLUMN_HEADER); + addMapping(map, Downloads.Impl.RequestHeaders.COLUMN_VALUE); } - private static final List downloadManagerColumnsList = - Arrays.asList(DownloadManager.UNDERLYING_COLUMNS); @VisibleForTesting SystemFacade mSystemFacade; @@ -918,28 +973,6 @@ public final class DownloadProvider extends ContentProvider { return qb.query(db, projection, null, null, null, null, null); } - if (shouldRestrictVisibility()) { - if (projection == null) { - projection = sAppReadableColumnsArray.clone(); - } else { - // check the validity of the columns in projection - for (int i = 0; i < projection.length; ++i) { - if (!sAppReadableColumnsSet.contains(projection[i]) && - !downloadManagerColumnsList.contains(projection[i])) { - throw new IllegalArgumentException( - "column " + projection[i] + " is not allowed in queries"); - } - } - } - - for (int i = 0; i < projection.length; i++) { - final String newColumn = sColumnsMap.get(projection[i]); - if (newColumn != null) { - projection[i] = newColumn; - } - } - } - if (Constants.LOGVV) { logVerboseQueryInfo(projection, selection, selectionArgs, sort, db); } @@ -1030,26 +1063,12 @@ public final class DownloadProvider extends ContentProvider { } } - /** - * @return true if we should restrict the columns readable by this caller - */ - private boolean shouldRestrictVisibility() { - int callingUid = Binder.getCallingUid(); - return Binder.getCallingPid() != Process.myPid() && - callingUid != mSystemUid && - callingUid != mDefContainerUid; - } - /** * Updates a row in the database */ @Override public int update(final Uri uri, final ContentValues values, final String where, final String[] whereArgs) { - if (shouldRestrictVisibility()) { - Helpers.validateSelection(where, sAppReadableColumnsSet); - } - final Context context = getContext(); final ContentResolver resolver = context.getContentResolver(); @@ -1168,6 +1187,8 @@ public final class DownloadProvider extends ContentProvider { */ private SQLiteQueryBuilder getQueryBuilder(final Uri uri, int match) { final String table; + final Map projectionMap; + final StringBuilder where = new StringBuilder(); switch (match) { // The "my_downloads" view normally limits the caller to operating @@ -1178,6 +1199,7 @@ public final class DownloadProvider extends ContentProvider { // fall-through case MY_DOWNLOADS: table = DB_TABLE; + projectionMap = sDownloadsMap; if (getContext().checkCallingOrSelfPermission( PERMISSION_ACCESS_ALL) != PackageManager.PERMISSION_GRANTED) { appendWhereExpression(where, Constants.UID + "=" + Binder.getCallingUid() @@ -1193,6 +1215,7 @@ public final class DownloadProvider extends ContentProvider { // fall-through case ALL_DOWNLOADS: table = DB_TABLE; + projectionMap = sDownloadsMap; break; // Headers are limited to callers holding the ACCESS_ALL_DOWNLOADS @@ -1200,6 +1223,7 @@ public final class DownloadProvider extends ContentProvider { case MY_DOWNLOADS_ID_HEADERS: case ALL_DOWNLOADS_ID_HEADERS: table = Downloads.Impl.RequestHeaders.HEADERS_DB_TABLE; + projectionMap = sHeadersMap; appendWhereExpression(where, Downloads.Impl.RequestHeaders.COLUMN_DOWNLOAD_ID + "=" + getDownloadIdFromUri(uri)); break; @@ -1209,8 +1233,11 @@ public final class DownloadProvider extends ContentProvider { } final SQLiteQueryBuilder qb = new SQLiteQueryBuilder(); - qb.setStrict(true); qb.setTables(table); + qb.setProjectionMap(projectionMap); + qb.setStrict(true); + qb.setStrictColumns(true); + qb.setStrictGrammar(true); qb.appendWhere(where); return qb; } @@ -1227,10 +1254,6 @@ public final class DownloadProvider extends ContentProvider { */ @Override public int delete(final Uri uri, final String where, final String[] whereArgs) { - if (shouldRestrictVisibility()) { - Helpers.validateSelection(where, sAppReadableColumnsSet); - } - final Context context = getContext(); final ContentResolver resolver = context.getContentResolver(); final JobScheduler scheduler = context.getSystemService(JobScheduler.class); diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java index 963ca9da..87cf0467 100644 --- a/src/com/android/providers/downloads/Helpers.java +++ b/src/com/android/providers/downloads/Helpers.java @@ -50,7 +50,7 @@ import com.google.common.annotations.VisibleForTesting; import java.io.File; import java.io.IOException; import java.util.Random; -import java.util.Set; +import java.util.function.BiConsumer; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -580,265 +580,4 @@ public class Helpers { throw new IllegalStateException("unexpected destination: " + destination); } } - - /** - * Checks whether this looks like a legitimate selection parameter - */ - public static void validateSelection(String selection, Set allowedColumns) { - try { - if (selection == null || selection.isEmpty()) { - return; - } - Lexer lexer = new Lexer(selection, allowedColumns); - parseExpression(lexer); - if (lexer.currentToken() != Lexer.TOKEN_END) { - throw new IllegalArgumentException("syntax error"); - } - } catch (RuntimeException ex) { - if (Constants.LOGV) { - Log.d(Constants.TAG, "invalid selection [" + selection + "] triggered " + ex); - } else if (false) { - Log.d(Constants.TAG, "invalid selection triggered " + ex); - } - throw ex; - } - - } - - // expression <- ( expression ) | statement [AND_OR ( expression ) | statement] * - // | statement [AND_OR expression]* - private static void parseExpression(Lexer lexer) { - for (;;) { - // ( expression ) - if (lexer.currentToken() == Lexer.TOKEN_OPEN_PAREN) { - lexer.advance(); - parseExpression(lexer); - if (lexer.currentToken() != Lexer.TOKEN_CLOSE_PAREN) { - throw new IllegalArgumentException("syntax error, unmatched parenthese"); - } - lexer.advance(); - } else { - // statement - parseStatement(lexer); - } - if (lexer.currentToken() != Lexer.TOKEN_AND_OR) { - break; - } - lexer.advance(); - } - } - - // statement <- COLUMN COMPARE VALUE - // | COLUMN IS NULL - private static void parseStatement(Lexer lexer) { - // both possibilities start with COLUMN - if (lexer.currentToken() != Lexer.TOKEN_COLUMN) { - throw new IllegalArgumentException("syntax error, expected column name"); - } - lexer.advance(); - - // statement <- COLUMN COMPARE VALUE - if (lexer.currentToken() == Lexer.TOKEN_COMPARE) { - lexer.advance(); - if (lexer.currentToken() != Lexer.TOKEN_VALUE) { - throw new IllegalArgumentException("syntax error, expected quoted string"); - } - lexer.advance(); - return; - } - - // statement <- COLUMN IS NULL - if (lexer.currentToken() == Lexer.TOKEN_IS) { - lexer.advance(); - if (lexer.currentToken() != Lexer.TOKEN_NULL) { - throw new IllegalArgumentException("syntax error, expected NULL"); - } - lexer.advance(); - return; - } - - // didn't get anything good after COLUMN - throw new IllegalArgumentException("syntax error after column name"); - } - - /** - * A simple lexer that recognizes the words of our restricted subset of SQL where clauses - */ - private static class Lexer { - public static final int TOKEN_START = 0; - public static final int TOKEN_OPEN_PAREN = 1; - public static final int TOKEN_CLOSE_PAREN = 2; - public static final int TOKEN_AND_OR = 3; - public static final int TOKEN_COLUMN = 4; - public static final int TOKEN_COMPARE = 5; - public static final int TOKEN_VALUE = 6; - public static final int TOKEN_IS = 7; - public static final int TOKEN_NULL = 8; - public static final int TOKEN_END = 9; - - private final String mSelection; - private final Set mAllowedColumns; - private int mOffset = 0; - private int mCurrentToken = TOKEN_START; - private final char[] mChars; - - public Lexer(String selection, Set allowedColumns) { - mSelection = selection; - mAllowedColumns = allowedColumns; - mChars = new char[mSelection.length()]; - mSelection.getChars(0, mChars.length, mChars, 0); - advance(); - } - - public int currentToken() { - return mCurrentToken; - } - - public void advance() { - char[] chars = mChars; - - // consume whitespace - while (mOffset < chars.length && chars[mOffset] == ' ') { - ++mOffset; - } - - // end of input - if (mOffset == chars.length) { - mCurrentToken = TOKEN_END; - return; - } - - // "(" - if (chars[mOffset] == '(') { - ++mOffset; - mCurrentToken = TOKEN_OPEN_PAREN; - return; - } - - // ")" - if (chars[mOffset] == ')') { - ++mOffset; - mCurrentToken = TOKEN_CLOSE_PAREN; - return; - } - - // "?" - if (chars[mOffset] == '?') { - ++mOffset; - mCurrentToken = TOKEN_VALUE; - return; - } - - // "=" and "==" - if (chars[mOffset] == '=') { - ++mOffset; - mCurrentToken = TOKEN_COMPARE; - if (mOffset < chars.length && chars[mOffset] == '=') { - ++mOffset; - } - return; - } - - // ">" and ">=" - if (chars[mOffset] == '>') { - ++mOffset; - mCurrentToken = TOKEN_COMPARE; - if (mOffset < chars.length && chars[mOffset] == '=') { - ++mOffset; - } - return; - } - - // "<", "<=" and "<>" - if (chars[mOffset] == '<') { - ++mOffset; - mCurrentToken = TOKEN_COMPARE; - if (mOffset < chars.length && (chars[mOffset] == '=' || chars[mOffset] == '>')) { - ++mOffset; - } - return; - } - - // "!=" - if (chars[mOffset] == '!') { - ++mOffset; - mCurrentToken = TOKEN_COMPARE; - if (mOffset < chars.length && chars[mOffset] == '=') { - ++mOffset; - return; - } - throw new IllegalArgumentException("Unexpected character after !"); - } - - // columns and keywords - // first look for anything that looks like an identifier or a keyword - // and then recognize the individual words. - // no attempt is made at discarding sequences of underscores with no alphanumeric - // characters, even though it's not clear that they'd be legal column names. - if (isIdentifierStart(chars[mOffset])) { - int startOffset = mOffset; - ++mOffset; - while (mOffset < chars.length && isIdentifierChar(chars[mOffset])) { - ++mOffset; - } - String word = mSelection.substring(startOffset, mOffset); - if (mOffset - startOffset <= 4) { - if (word.equals("IS")) { - mCurrentToken = TOKEN_IS; - return; - } - if (word.equals("OR") || word.equals("AND")) { - mCurrentToken = TOKEN_AND_OR; - return; - } - if (word.equals("NULL")) { - mCurrentToken = TOKEN_NULL; - return; - } - } - if (mAllowedColumns.contains(word)) { - mCurrentToken = TOKEN_COLUMN; - return; - } - throw new IllegalArgumentException("unrecognized column or keyword: " + word); - } - - // quoted strings - if (chars[mOffset] == '\'') { - ++mOffset; - while (mOffset < chars.length) { - if (chars[mOffset] == '\'') { - if (mOffset + 1 < chars.length && chars[mOffset + 1] == '\'') { - ++mOffset; - } else { - break; - } - } - ++mOffset; - } - if (mOffset == chars.length) { - throw new IllegalArgumentException("unterminated string"); - } - ++mOffset; - mCurrentToken = TOKEN_VALUE; - return; - } - - // anything we don't recognize - throw new IllegalArgumentException("illegal character: " + chars[mOffset]); - } - - private static final boolean isIdentifierStart(char c) { - return c == '_' || - (c >= 'A' && c <= 'Z') || - (c >= 'a' && c <= 'z'); - } - - private static final boolean isIdentifierChar(char c) { - return c == '_' || - (c >= 'A' && c <= 'Z') || - (c >= 'a' && c <= 'z') || - (c >= '0' && c <= '9'); - } - } } -- cgit v1.2.3