Validate user-supplied URIs in DocumentsProvider calls

Some URIs are used without validating their authorities which can lead
to exploitation by malicious apps.

Bug: 157294893
Test: Manual using test app in b/157294893
Change-Id: I799509ed5ff7e69140e84d796fe7f96d9dbfd32f
This commit is contained in:
Abhijeet Kaur
2020-08-13 12:21:15 +01:00
parent e0f4a9bb6c
commit 75f984bd32

View File

@@ -239,6 +239,10 @@ public abstract class DocumentsProvider extends ContentProvider {
} }
} }
private Uri validateIncomingNullableUri(@Nullable Uri uri) {
return uri == null ? null : validateIncomingUri(uri);
}
/** /**
* Create a new document and return its newly generated * Create a new document and return its newly generated
* {@link Document#COLUMN_DOCUMENT_ID}. You must allocate a new * {@link Document#COLUMN_DOCUMENT_ID}. You must allocate a new
@@ -1086,11 +1090,18 @@ public abstract class DocumentsProvider extends ContentProvider {
// If the URI is a tree URI performs some validation. // If the URI is a tree URI performs some validation.
enforceTreeForExtraUris(extras); enforceTreeForExtraUris(extras);
final Uri extraUri = validateIncomingNullableUri(
extras.getParcelable(DocumentsContract.EXTRA_URI));
final Uri extraTargetUri = validateIncomingNullableUri(
extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI));
final Uri extraParentUri = validateIncomingNullableUri(
extras.getParcelable(DocumentsContract.EXTRA_PARENT_URI));
if (METHOD_EJECT_ROOT.equals(method)) { if (METHOD_EJECT_ROOT.equals(method)) {
// Given that certain system apps can hold MOUNT_UNMOUNT permission, but only apps // Given that certain system apps can hold MOUNT_UNMOUNT permission, but only apps
// signed with platform signature can hold MANAGE_DOCUMENTS, we are going to check for // signed with platform signature can hold MANAGE_DOCUMENTS, we are going to check for
// MANAGE_DOCUMENTS or associated URI permission here instead // MANAGE_DOCUMENTS or associated URI permission here instead
final Uri rootUri = extras.getParcelable(DocumentsContract.EXTRA_URI); final Uri rootUri = extraUri;
enforceWritePermissionInner(rootUri, getCallingPackage(), getCallingAttributionTag(), enforceWritePermissionInner(rootUri, getCallingPackage(), getCallingAttributionTag(),
null); null);
@@ -1100,7 +1111,7 @@ public abstract class DocumentsProvider extends ContentProvider {
return out; return out;
} }
final Uri documentUri = extras.getParcelable(DocumentsContract.EXTRA_URI); final Uri documentUri = extraUri;
final String authority = documentUri.getAuthority(); final String authority = documentUri.getAuthority();
final String documentId = DocumentsContract.getDocumentId(documentUri); final String documentId = DocumentsContract.getDocumentId(documentUri);
@@ -1113,7 +1124,7 @@ public abstract class DocumentsProvider extends ContentProvider {
enforceReadPermissionInner(documentUri, getCallingPackage(), enforceReadPermissionInner(documentUri, getCallingPackage(),
getCallingAttributionTag(), null); getCallingAttributionTag(), null);
final Uri childUri = extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI); final Uri childUri = extraTargetUri;
final String childAuthority = childUri.getAuthority(); final String childAuthority = childUri.getAuthority();
final String childId = DocumentsContract.getDocumentId(childUri); final String childId = DocumentsContract.getDocumentId(childUri);
@@ -1180,7 +1191,7 @@ public abstract class DocumentsProvider extends ContentProvider {
revokeDocumentPermission(documentId); revokeDocumentPermission(documentId);
} else if (METHOD_COPY_DOCUMENT.equals(method)) { } else if (METHOD_COPY_DOCUMENT.equals(method)) {
final Uri targetUri = extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI); final Uri targetUri = extraTargetUri;
final String targetId = DocumentsContract.getDocumentId(targetUri); final String targetId = DocumentsContract.getDocumentId(targetUri);
enforceReadPermissionInner(documentUri, getCallingPackage(), enforceReadPermissionInner(documentUri, getCallingPackage(),
@@ -1204,9 +1215,9 @@ public abstract class DocumentsProvider extends ContentProvider {
} }
} else if (METHOD_MOVE_DOCUMENT.equals(method)) { } else if (METHOD_MOVE_DOCUMENT.equals(method)) {
final Uri parentSourceUri = extras.getParcelable(DocumentsContract.EXTRA_PARENT_URI); final Uri parentSourceUri = extraParentUri;
final String parentSourceId = DocumentsContract.getDocumentId(parentSourceUri); final String parentSourceId = DocumentsContract.getDocumentId(parentSourceUri);
final Uri targetUri = extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI); final Uri targetUri = extraTargetUri;
final String targetId = DocumentsContract.getDocumentId(targetUri); final String targetId = DocumentsContract.getDocumentId(targetUri);
enforceWritePermissionInner(documentUri, getCallingPackage(), enforceWritePermissionInner(documentUri, getCallingPackage(),
@@ -1232,7 +1243,7 @@ public abstract class DocumentsProvider extends ContentProvider {
} }
} else if (METHOD_REMOVE_DOCUMENT.equals(method)) { } else if (METHOD_REMOVE_DOCUMENT.equals(method)) {
final Uri parentSourceUri = extras.getParcelable(DocumentsContract.EXTRA_PARENT_URI); final Uri parentSourceUri = extraParentUri;
final String parentSourceId = DocumentsContract.getDocumentId(parentSourceUri); final String parentSourceId = DocumentsContract.getDocumentId(parentSourceUri);
enforceReadPermissionInner(parentSourceUri, getCallingPackage(), enforceReadPermissionInner(parentSourceUri, getCallingPackage(),