Recover shady content:// paths.
The path-permission element offers prefix or regex style matching of
paths, but most providers internally use UriMatcher to decide what
to do with an incoming Uri.
This causes trouble because UriMatcher uses Uri.getPathSegments(),
which quietly ignores "empty" paths. Consider this example:
<path-permission android:pathPrefix="/private" ... />
uriMatcher.addURI("com.example", "/private", CODE_PRIVATE);
content://com.example//private
The Uri above will pass the security check, since it's not
technically a prefix match. But the UriMatcher will then match it
as CODE_PRIVATE, since it ignores the "//" zero-length path.
Since we can't safely change the behavior of either path-permission
or UriMatcher, we're left with recovering these shady paths by
trimming away zero-length paths.
Bug: 112555574
Test: atest android.appsecurity.cts.AppSecurityTests
Test: atest FrameworksCoreTests:android.content.ContentProviderTest
Change-Id: Ibadbfa4fc904ec54780c8102958735b03293fb9a
diff --git a/core/java/android/content/ContentProvider.java b/core/java/android/content/ContentProvider.java
index bd1e6a4..dd9370d 100644
--- a/core/java/android/content/ContentProvider.java
+++ b/core/java/android/content/ContentProvider.java
@@ -51,6 +51,8 @@
import android.text.TextUtils;
import android.util.Log;
+import com.android.internal.annotations.VisibleForTesting;
+
import java.io.File;
import java.io.FileDescriptor;
import java.io.FileNotFoundException;
@@ -58,6 +60,7 @@
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Objects;
/**
* Content providers are one of the primary building blocks of Android applications, providing
@@ -220,7 +223,7 @@
@Override
public Cursor query(String callingPkg, Uri uri, @Nullable String[] projection,
@Nullable Bundle queryArgs, @Nullable ICancellationSignal cancellationSignal) {
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
uri = maybeGetUriWithoutUserId(uri);
if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
// The caller has no access to the data, so return an empty cursor with
@@ -268,7 +271,7 @@
@Override
public String getType(Uri uri) {
// getCallingPackage() isn't available in getType(), as the javadoc states.
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
uri = maybeGetUriWithoutUserId(uri);
Trace.traceBegin(TRACE_TAG_DATABASE, "getType");
try {
@@ -280,7 +283,7 @@
@Override
public Uri insert(String callingPkg, Uri uri, ContentValues initialValues) {
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
int userId = getUserIdFromUri(uri);
uri = maybeGetUriWithoutUserId(uri);
if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
@@ -303,7 +306,7 @@
@Override
public int bulkInsert(String callingPkg, Uri uri, ContentValues[] initialValues) {
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
uri = maybeGetUriWithoutUserId(uri);
if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
return 0;
@@ -327,11 +330,11 @@
for (int i = 0; i < numOperations; i++) {
ContentProviderOperation operation = operations.get(i);
Uri uri = operation.getUri();
- validateIncomingUri(uri);
- userIds[i] = getUserIdFromUri(uri);
- if (userIds[i] != UserHandle.USER_CURRENT) {
- // Removing the user id from the uri.
- operation = new ContentProviderOperation(operation, true);
+ uri = validateIncomingUri(uri);
+ uri = maybeGetUriWithoutUserId(uri);
+ // Rebuild operation if we changed the Uri above
+ if (!Objects.equals(operation.getUri(), uri)) {
+ operation = new ContentProviderOperation(operation, uri);
operations.set(i, operation);
}
if (operation.isReadOperation()) {
@@ -368,7 +371,7 @@
@Override
public int delete(String callingPkg, Uri uri, String selection, String[] selectionArgs) {
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
uri = maybeGetUriWithoutUserId(uri);
if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
return 0;
@@ -386,7 +389,7 @@
@Override
public int update(String callingPkg, Uri uri, ContentValues values, String selection,
String[] selectionArgs) {
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
uri = maybeGetUriWithoutUserId(uri);
if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
return 0;
@@ -405,7 +408,7 @@
public ParcelFileDescriptor openFile(
String callingPkg, Uri uri, String mode, ICancellationSignal cancellationSignal,
IBinder callerToken) throws FileNotFoundException {
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
uri = maybeGetUriWithoutUserId(uri);
enforceFilePermission(callingPkg, uri, mode, callerToken);
Trace.traceBegin(TRACE_TAG_DATABASE, "openFile");
@@ -423,7 +426,7 @@
public AssetFileDescriptor openAssetFile(
String callingPkg, Uri uri, String mode, ICancellationSignal cancellationSignal)
throws FileNotFoundException {
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
uri = maybeGetUriWithoutUserId(uri);
enforceFilePermission(callingPkg, uri, mode, null);
Trace.traceBegin(TRACE_TAG_DATABASE, "openAssetFile");
@@ -454,7 +457,7 @@
@Override
public String[] getStreamTypes(Uri uri, String mimeTypeFilter) {
// getCallingPackage() isn't available in getType(), as the javadoc states.
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
uri = maybeGetUriWithoutUserId(uri);
Trace.traceBegin(TRACE_TAG_DATABASE, "getStreamTypes");
try {
@@ -468,7 +471,7 @@
public AssetFileDescriptor openTypedAssetFile(String callingPkg, Uri uri, String mimeType,
Bundle opts, ICancellationSignal cancellationSignal) throws FileNotFoundException {
Bundle.setDefusable(opts, true);
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
uri = maybeGetUriWithoutUserId(uri);
enforceFilePermission(callingPkg, uri, "r", null);
Trace.traceBegin(TRACE_TAG_DATABASE, "openTypedAssetFile");
@@ -489,7 +492,7 @@
@Override
public Uri canonicalize(String callingPkg, Uri uri) {
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
int userId = getUserIdFromUri(uri);
uri = getUriWithoutUserId(uri);
if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
@@ -507,7 +510,7 @@
@Override
public Uri uncanonicalize(String callingPkg, Uri uri) {
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
int userId = getUserIdFromUri(uri);
uri = getUriWithoutUserId(uri);
if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
@@ -526,7 +529,7 @@
@Override
public boolean refresh(String callingPkg, Uri uri, Bundle args,
ICancellationSignal cancellationSignal) throws RemoteException {
- validateIncomingUri(uri);
+ uri = validateIncomingUri(uri);
uri = getUriWithoutUserId(uri);
if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
return false;
@@ -1965,7 +1968,7 @@
*/
if (mContext == null) {
mContext = context;
- if (context != null) {
+ if (context != null && mTransport != null) {
mTransport.mAppOpsManager = (AppOpsManager) context.getSystemService(
Context.APP_OPS_SERVICE);
}
@@ -2074,7 +2077,8 @@
}
/** @hide */
- private void validateIncomingUri(Uri uri) throws SecurityException {
+ @VisibleForTesting
+ public Uri validateIncomingUri(Uri uri) throws SecurityException {
String auth = uri.getAuthority();
if (!mSingleUser) {
int userId = getUserIdFromAuthority(auth, UserHandle.USER_CURRENT);
@@ -2093,6 +2097,15 @@
}
throw new SecurityException(message);
}
+
+ // Normalize the path by removing any empty path segments, which can be
+ // a source of security issues.
+ final String encodedPath = uri.getEncodedPath();
+ if (encodedPath != null && encodedPath.indexOf("//") != -1) {
+ return uri.buildUpon().encodedPath(encodedPath.replaceAll("//+", "/")).build();
+ } else {
+ return uri;
+ }
}
/** @hide */