From e4bb1d7b6cd538acf423c7d8864dd26819fe8757 Mon Sep 17 00:00:00 2001 From: Abhijeet Kaur Date: Thu, 13 Aug 2020 12:21:15 +0100 Subject: [PATCH] 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 Merged-In: I799509ed5ff7e69140e84d796fe7f96d9dbfd32f (cherry picked from commit 75f984bd32a3ee8115d5cea09ab1bd237537ab54) --- .../android/provider/DocumentsProvider.java | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/core/java/android/provider/DocumentsProvider.java b/core/java/android/provider/DocumentsProvider.java index 327bca268a7bc..91b591c7b77ef 100644 --- a/core/java/android/provider/DocumentsProvider.java +++ b/core/java/android/provider/DocumentsProvider.java @@ -232,6 +232,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 * {@link Document#COLUMN_DOCUMENT_ID}. You must allocate a new @@ -1076,11 +1080,18 @@ public abstract class DocumentsProvider extends ContentProvider { final Context context = getContext(); final Bundle out = new Bundle(); + 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)) { // 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 // MANAGE_DOCUMENTS or associated URI permission here instead - final Uri rootUri = extras.getParcelable(DocumentsContract.EXTRA_URI); + final Uri rootUri = extraUri; enforceWritePermissionInner(rootUri, getCallingPackage(), getCallingAttributionTag(), null); @@ -1090,7 +1101,7 @@ public abstract class DocumentsProvider extends ContentProvider { return out; } - final Uri documentUri = extras.getParcelable(DocumentsContract.EXTRA_URI); + final Uri documentUri = extraUri; final String authority = documentUri.getAuthority(); final String documentId = DocumentsContract.getDocumentId(documentUri); @@ -1106,7 +1117,7 @@ public abstract class DocumentsProvider extends ContentProvider { enforceReadPermissionInner(documentUri, getCallingPackage(), getCallingAttributionTag(), null); - final Uri childUri = extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI); + final Uri childUri = extraTargetUri; final String childAuthority = childUri.getAuthority(); final String childId = DocumentsContract.getDocumentId(childUri); @@ -1173,7 +1184,7 @@ public abstract class DocumentsProvider extends ContentProvider { revokeDocumentPermission(documentId); } 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); enforceReadPermissionInner(documentUri, getCallingPackage(), @@ -1197,9 +1208,9 @@ public abstract class DocumentsProvider extends ContentProvider { } } 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 Uri targetUri = extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI); + final Uri targetUri = extraTargetUri; final String targetId = DocumentsContract.getDocumentId(targetUri); enforceWritePermissionInner(documentUri, getCallingPackage(), @@ -1225,7 +1236,7 @@ public abstract class DocumentsProvider extends ContentProvider { } } 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); enforceReadPermissionInner(parentSourceUri, getCallingPackage(),