Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

RESTRICT AUTOMERGE: Recover shady content:// paths.

Browse Source 
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: cts-tradefed run cts -m CtsAppSecurityHostTestCases -t android.appsecurity.cts.AppSecurityTests
Change-Id: Ibadbfa4fc904ec54780c8102958735b03293fb9a
This commit is contained in:
Jeff Sharkey
2018-09-24 13:23:57 -06:00
committed by Atanas Kirilov
parent 50708217f8
commit 301d17e4dd
2 changed files with 35 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
core/java/android/content/ContentProvider.java
View File

@@ -53,6 +53,7 @@ import java.io.IOException;
import java.io.PrintWriter; import java.io.PrintWriter;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Objects;
/** /**
* Content providers are one of the primary building blocks of Android applications, providing * Content providers are one of the primary building blocks of Android applications, providing
@@ -207,7 +208,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
public Cursor query(String callingPkg, Uri uri, String[] projection, public Cursor query(String callingPkg, Uri uri, String[] projection,
String selection, String[] selectionArgs, String sortOrder, String selection, String[] selectionArgs, String sortOrder,
ICancellationSignal cancellationSignal) { ICancellationSignal cancellationSignal) {
validateIncomingUri(uri); uri = validateIncomingUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) { if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
// The caller has no access to the data, so return an empty cursor with // The caller has no access to the data, so return an empty cursor with
@@ -246,14 +247,14 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
@Override @Override
public String getType(Uri uri) { public String getType(Uri uri) {
validateIncomingUri(uri); uri = validateIncomingUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
return ContentProvider.this.getType(uri); return ContentProvider.this.getType(uri);
} }
@Override @Override
public Uri insert(String callingPkg, Uri uri, ContentValues initialValues) { public Uri insert(String callingPkg, Uri uri, ContentValues initialValues) {
validateIncomingUri(uri); uri = validateIncomingUri(uri);
int userId = getUserIdFromUri(uri); int userId = getUserIdFromUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) { if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
@@ -269,7 +270,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
@Override @Override
public int bulkInsert(String callingPkg, Uri uri, ContentValues[] initialValues) { public int bulkInsert(String callingPkg, Uri uri, ContentValues[] initialValues) {
validateIncomingUri(uri); uri = validateIncomingUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) { if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
return 0; return 0;
@@ -291,11 +292,12 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
for (int i = 0; i < numOperations; i++) { for (int i = 0; i < numOperations; i++) {
ContentProviderOperation operation = operations.get(i); ContentProviderOperation operation = operations.get(i);
Uri uri = operation.getUri(); Uri uri = operation.getUri();
validateIncomingUri(uri);
userIds[i] = getUserIdFromUri(uri); userIds[i] = getUserIdFromUri(uri);
if (userIds[i] != UserHandle.USER_CURRENT) { uri = validateIncomingUri(uri);
// Removing the user id from the uri. uri = getUriWithoutUserId(uri);
operation = new ContentProviderOperation(operation, true); // Rebuild operation if we changed the Uri above
if (!Objects.equals(operation.getUri(), uri)) {
operation = new ContentProviderOperation(operation, uri);
operations.set(i, operation); operations.set(i, operation);
} }
if (operation.isReadOperation()) { if (operation.isReadOperation()) {
@@ -330,7 +332,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
@Override @Override
public int delete(String callingPkg, Uri uri, String selection, String[] selectionArgs) { public int delete(String callingPkg, Uri uri, String selection, String[] selectionArgs) {
validateIncomingUri(uri); uri = validateIncomingUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) { if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
return 0; return 0;
@@ -346,7 +348,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
@Override @Override
public int update(String callingPkg, Uri uri, ContentValues values, String selection, public int update(String callingPkg, Uri uri, ContentValues values, String selection,
String[] selectionArgs) { String[] selectionArgs) {
validateIncomingUri(uri); uri = validateIncomingUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) { if (enforceWritePermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
return 0; return 0;
@@ -363,7 +365,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
public ParcelFileDescriptor openFile( public ParcelFileDescriptor openFile(
String callingPkg, Uri uri, String mode, ICancellationSignal cancellationSignal, String callingPkg, Uri uri, String mode, ICancellationSignal cancellationSignal,
IBinder callerToken) throws FileNotFoundException { IBinder callerToken) throws FileNotFoundException {
validateIncomingUri(uri); uri = validateIncomingUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
enforceFilePermission(callingPkg, uri, mode, callerToken); enforceFilePermission(callingPkg, uri, mode, callerToken);
final String original = setCallingPackage(callingPkg); final String original = setCallingPackage(callingPkg);
@@ -379,7 +381,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
public AssetFileDescriptor openAssetFile( public AssetFileDescriptor openAssetFile(
String callingPkg, Uri uri, String mode, ICancellationSignal cancellationSignal) String callingPkg, Uri uri, String mode, ICancellationSignal cancellationSignal)
throws FileNotFoundException { throws FileNotFoundException {
validateIncomingUri(uri); uri = validateIncomingUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
enforceFilePermission(callingPkg, uri, mode, null); enforceFilePermission(callingPkg, uri, mode, null);
final String original = setCallingPackage(callingPkg); final String original = setCallingPackage(callingPkg);
@@ -405,7 +407,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
@Override @Override
public String[] getStreamTypes(Uri uri, String mimeTypeFilter) { public String[] getStreamTypes(Uri uri, String mimeTypeFilter) {
validateIncomingUri(uri); uri = validateIncomingUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
return ContentProvider.this.getStreamTypes(uri, mimeTypeFilter); return ContentProvider.this.getStreamTypes(uri, mimeTypeFilter);
} }
@@ -414,7 +416,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
public AssetFileDescriptor openTypedAssetFile(String callingPkg, Uri uri, String mimeType, public AssetFileDescriptor openTypedAssetFile(String callingPkg, Uri uri, String mimeType,
Bundle opts, ICancellationSignal cancellationSignal) throws FileNotFoundException { Bundle opts, ICancellationSignal cancellationSignal) throws FileNotFoundException {
Bundle.setDefusable(opts, true); Bundle.setDefusable(opts, true);
validateIncomingUri(uri); uri = validateIncomingUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
enforceFilePermission(callingPkg, uri, "r", null); enforceFilePermission(callingPkg, uri, "r", null);
final String original = setCallingPackage(callingPkg); final String original = setCallingPackage(callingPkg);
@@ -433,7 +435,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
@Override @Override
public Uri canonicalize(String callingPkg, Uri uri) { public Uri canonicalize(String callingPkg, Uri uri) {
validateIncomingUri(uri); uri = validateIncomingUri(uri);
int userId = getUserIdFromUri(uri); int userId = getUserIdFromUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) { if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
@@ -449,7 +451,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
@Override @Override
public Uri uncanonicalize(String callingPkg, Uri uri) { public Uri uncanonicalize(String callingPkg, Uri uri) {
validateIncomingUri(uri); uri = validateIncomingUri(uri);
int userId = getUserIdFromUri(uri); int userId = getUserIdFromUri(uri);
uri = getUriWithoutUserId(uri); uri = getUriWithoutUserId(uri);
if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) { if (enforceReadPermission(callingPkg, uri, null) != AppOpsManager.MODE_ALLOWED) {
@@ -1735,7 +1737,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
*/ */
if (mContext == null) { if (mContext == null) {
mContext = context; mContext = context;
if (context != null) { if (context != null && mTransport != null) {
mTransport.mAppOpsManager = (AppOpsManager) context.getSystemService( mTransport.mAppOpsManager = (AppOpsManager) context.getSystemService(
Context.APP_OPS_SERVICE); Context.APP_OPS_SERVICE);
} }
@@ -1844,7 +1846,7 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
} }
/** @hide */ /** @hide */
private void validateIncomingUri(Uri uri) throws SecurityException { public Uri validateIncomingUri(Uri uri) throws SecurityException {
String auth = uri.getAuthority(); String auth = uri.getAuthority();
int userId = getUserIdFromAuthority(auth, UserHandle.USER_CURRENT); int userId = getUserIdFromAuthority(auth, UserHandle.USER_CURRENT);
if (userId != UserHandle.USER_CURRENT && userId != mContext.getUserId()) { if (userId != UserHandle.USER_CURRENT && userId != mContext.getUserId()) {
@@ -1861,6 +1863,19 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
} }
throw new SecurityException(message); 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) {
final Uri normalized = uri.buildUpon()
.encodedPath(encodedPath.replaceAll("//+", "/")).build();
Log.w(TAG, "Normalized " + uri + " to " + normalized
+ " to avoid possible security issues");
return normalized;
} else {
return uri;
}
} }
/** @hide */ /** @hide */

16
core/java/android/content/ContentProviderOperation.java
View File

@@ -94,13 +94,9 @@ public class ContentProviderOperation implements Parcelable {
} }
/** @hide */ /** @hide */
public ContentProviderOperation(ContentProviderOperation cpo, boolean removeUserIdFromUri) { public ContentProviderOperation(ContentProviderOperation cpo, Uri withUri) {
mType = cpo.mType; mType = cpo.mType;
if (removeUserIdFromUri) { mUri = withUri;
mUri = ContentProvider.getUriWithoutUserId(cpo.mUri);
} else {
mUri = cpo.mUri;
}
mValues = cpo.mValues; mValues = cpo.mValues;
mSelection = cpo.mSelection; mSelection = cpo.mSelection;
mSelectionArgs = cpo.mSelectionArgs; mSelectionArgs = cpo.mSelectionArgs;
@@ -110,14 +106,6 @@ public class ContentProviderOperation implements Parcelable {
mYieldAllowed = cpo.mYieldAllowed; mYieldAllowed = cpo.mYieldAllowed;
} }
/** @hide */
public ContentProviderOperation getWithoutUserIdInUri() {
if (ContentProvider.uriHasUserId(mUri)) {
return new ContentProviderOperation(this, true);
}
return this;
}
public void writeToParcel(Parcel dest, int flags) { public void writeToParcel(Parcel dest, int flags) {
dest.writeInt(mType); dest.writeInt(mType);
Uri.writeToParcel(dest, mUri); Uri.writeToParcel(dest, mUri);