From b9f4fb792812f9a38ac54e69be6f121f7367c017 Mon Sep 17 00:00:00 2001 From: Abhijeet Kaur Date: Wed, 12 Aug 2020 17:34:22 +0100 Subject: [PATCH] Validate user-supplied tree URIs in DocumentsProvider calls Currently we only validate DocumentsContract.EXTRA_URI, this change validates other URIs suchs as DocumentsContract.EXTRA_TARGET_URI and DocumentsContract.EXTRA_PARENT_URI as well Bug: 157320716 Test: Manually using the test app in b/157320716#comment1 Change-Id: I90fd1e62aa7dc333bf32eb80ccc5b181a1d54e41 --- .../android/provider/DocumentsProvider.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/core/java/android/provider/DocumentsProvider.java b/core/java/android/provider/DocumentsProvider.java index 327bca268a7bc..d55fc511fc778 100644 --- a/core/java/android/provider/DocumentsProvider.java +++ b/core/java/android/provider/DocumentsProvider.java @@ -218,8 +218,15 @@ public abstract class DocumentsProvider extends ContentProvider { } /** {@hide} */ - private void enforceTree(Uri documentUri) { - if (isTreeUri(documentUri)) { + private void enforceTreeForExtraUris(Bundle extras) { + enforceTree(extras.getParcelable(DocumentsContract.EXTRA_URI)); + enforceTree(extras.getParcelable(DocumentsContract.EXTRA_PARENT_URI)); + enforceTree(extras.getParcelable(DocumentsContract.EXTRA_TARGET_URI)); + } + + /** {@hide} */ + private void enforceTree(@Nullable Uri documentUri) { + if (documentUri != null && isTreeUri(documentUri)) { final String parent = getTreeDocumentId(documentUri); final String child = getDocumentId(documentUri); if (Objects.equals(parent, child)) { @@ -1076,6 +1083,9 @@ public abstract class DocumentsProvider extends ContentProvider { final Context context = getContext(); final Bundle out = new Bundle(); + // If the URI is a tree URI performs some validation. + enforceTreeForExtraUris(extras); + 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 @@ -1099,9 +1109,6 @@ public abstract class DocumentsProvider extends ContentProvider { "Requested authority " + authority + " doesn't match provider " + mAuthority); } - // If the URI is a tree URI performs some validation. - enforceTree(documentUri); - if (METHOD_IS_CHILD_DOCUMENT.equals(method)) { enforceReadPermissionInner(documentUri, getCallingPackage(), getCallingAttributionTag(), null);