Merge "Filter custom chooser actions for invalid Uris" into udc-dev am: d6a6ea70bb am: b8cb8440db

Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/modules/IntentResolver/+/22919057

Change-Id: I6ef59feb84df9f30935b4c696751f299f91dfc37
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

6 files changed, 170 insertions, 27 deletions

diff --git a/java/src/com/android/intentresolver/ChooserRequestParameters.java b/java/src/com/android/intentresolver/ChooserRequestParameters.java
index 039f50e9..b3f5a722 100644
--- a/java/src/com/android/intentresolver/ChooserRequestParameters.java
+++ b/java/src/com/android/intentresolver/ChooserRequestParameters.java
@@ -33,6 +33,7 @@ import android.util.Log;
 import android.util.Pair;
 
 import com.android.intentresolver.flags.FeatureFlagRepository;
+import com.android.intentresolver.util.UriFilters;
 
 import com.google.common.collect.ImmutableList;
 
@@ -320,7 +321,8 @@ public class ChooserRequestParameters {
                 ChooserAction.class,
                 true,
                 true)
-            .collect(toImmutableList());
+                .filter(UriFilters::hasValidIcon)
+                .collect(toImmutableList());
     }
 
     @Nullable
diff --git a/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java
index 8173d542..e9476909 100644
--- a/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java
+++ b/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java
@@ -19,6 +19,7 @@ package com.android.intentresolver.contentpreview;
 import static android.provider.DocumentsContract.Document.FLAG_SUPPORTS_THUMBNAIL;
 
 import static com.android.intentresolver.contentpreview.ContentPreviewType.CONTENT_PREVIEW_IMAGE;
+import static com.android.intentresolver.util.UriFilters.isOwnedByCurrentUser;
 
 import android.content.ClipData;
 import android.content.ClipDescription;
@@ -319,14 +320,14 @@ public final class ChooserContentPreviewUi {
         List<Uri> uris = new ArrayList<>();
         if (Intent.ACTION_SEND.equals(targetIntent.getAction())) {
             Uri uri = targetIntent.getParcelableExtra(Intent.EXTRA_STREAM);
-            if (ContentPreviewUi.validForContentPreview(uri)) {
+            if (isOwnedByCurrentUser(uri)) {
                 uris.add(uri);
             }
         } else {
             List<Uri> receivedUris = targetIntent.getParcelableArrayListExtra(Intent.EXTRA_STREAM);
             if (receivedUris != null) {
                 for (Uri uri : receivedUris) {
-                    if (ContentPreviewUi.validForContentPreview(uri)) {
+                    if (isOwnedByCurrentUser(uri)) {
                         uris.add(uri);
                     }
                 }
diff --git a/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java
index 4e343a17..c0859e53 100644
--- a/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java
+++ b/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java
@@ -16,16 +16,11 @@
 
 package com.android.intentresolver.contentpreview;
 
-import static android.content.ContentProvider.getUserIdFromUri;
-
 import android.animation.ObjectAnimator;
 import android.animation.ValueAnimator;
 import android.content.res.Resources;
 import android.graphics.Bitmap;
-import android.net.Uri;
-import android.os.UserHandle;
 import android.text.TextUtils;
-import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
@@ -63,24 +58,6 @@ abstract class ContentPreviewUi {
         return actions;
     }
 
-    /**
-     * Indicate if the incoming content URI should be allowed.
-     *
-     * @param uri the uri to test
-     * @return true if the URI is allowed for content preview
-     */
-    protected static boolean validForContentPreview(Uri uri) throws SecurityException {
-        if (uri == null) {
-            return false;
-        }
-        int userId = getUserIdFromUri(uri, UserHandle.USER_CURRENT);
-        if (userId != UserHandle.USER_CURRENT && userId != UserHandle.myUserId()) {
-            Log.e(ContentPreviewUi.TAG, "dropped invalid content URI belonging to user " + userId);
-            return false;
-        }
-        return true;
-    }
-
     protected static void updateViewWithImage(ImageView imageView, Bitmap image) {
         if (image == null) {
             imageView.setVisibility(View.GONE);
diff --git a/java/src/com/android/intentresolver/contentpreview/TextContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/TextContentPreviewUi.java
index ccf5061d..c429b2d6 100644
--- a/java/src/com/android/intentresolver/contentpreview/TextContentPreviewUi.java
+++ b/java/src/com/android/intentresolver/contentpreview/TextContentPreviewUi.java
@@ -16,6 +16,8 @@
 
 package com.android.intentresolver.contentpreview;
 
+import static com.android.intentresolver.util.UriFilters.isOwnedByCurrentUser;
+
 import android.content.res.Resources;
 import android.net.Uri;
 import android.text.TextUtils;
@@ -114,7 +116,7 @@ class TextContentPreviewUi extends ContentPreviewUi {
 
         ImageView previewThumbnailView = contentPreviewLayout.findViewById(
                 com.android.internal.R.id.content_preview_thumbnail);
-        if (!validForContentPreview(mPreviewThumbnail) || minimalPreview) {
+        if (!isOwnedByCurrentUser(mPreviewThumbnail) || minimalPreview) {
             previewThumbnailView.setVisibility(View.GONE);
         } else {
             mImageLoader.loadImage(
diff --git a/java/src/com/android/intentresolver/util/UriFilters.kt b/java/src/com/android/intentresolver/util/UriFilters.kt
new file mode 100644
index 00000000..8714c314
--- /dev/null
+++ b/java/src/com/android/intentresolver/util/UriFilters.kt
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+@file:JvmName("UriFilters")
+
+package com.android.intentresolver.util
+
+import android.content.ContentProvider.getUserIdFromUri
+import android.content.ContentResolver.SCHEME_CONTENT
+import android.graphics.drawable.Icon.TYPE_URI
+import android.graphics.drawable.Icon.TYPE_URI_ADAPTIVE_BITMAP
+import android.net.Uri
+import android.os.UserHandle
+import android.service.chooser.ChooserAction
+
+/**
+ * Checks if the [Uri] is a `content://` uri which references the current user (from process uid).
+ *
+ * MediaStore interprets the user field of a content:// URI as a UserId and applies it if the caller
+ * holds INTERACT_ACROSS_USERS permission. (Example: `content://10@media/images/1234`)
+ *
+ * No URI content should be loaded unless it passes this check since the caller would not have
+ * permission to read it.
+ *
+ * @return false if this is a content:// [Uri] which references another user
+ */
+val Uri?.ownedByCurrentUser: Boolean
+    @JvmName("isOwnedByCurrentUser")
+    get() = this?.let {
+        when (getUserIdFromUri(this, UserHandle.USER_CURRENT)) {
+            UserHandle.USER_CURRENT,
+            UserHandle.myUserId() -> true
+            else -> false
+        }
+    } == true
+
+/** Does the [Uri] reference a content provider ('content://')? */
+internal val Uri.contentScheme: Boolean
+    get() = scheme == SCHEME_CONTENT
+
+/**
+ * Checks if the Icon of a [ChooserAction] backed by content:// [Uri] is safe for display.
+ *
+ * @param action the chooser action
+ * @see [Uri.ownedByCurrentUser]
+ */
+fun hasValidIcon(action: ChooserAction) =
+    with(action.icon) {
+        when (type) {
+            TYPE_URI,
+            TYPE_URI_ADAPTIVE_BITMAP -> !uri.contentScheme || uri.ownedByCurrentUser
+            else -> true
+        }
+    }
diff --git a/java/tests/src/com/android/intentresolver/util/UriFiltersTest.kt b/java/tests/src/com/android/intentresolver/util/UriFiltersTest.kt
new file mode 100644
index 00000000..18218064
--- /dev/null
+++ b/java/tests/src/com/android/intentresolver/util/UriFiltersTest.kt
@@ -0,0 +1,95 @@
+package com.android.intentresolver.util
+
+import android.app.PendingIntent
+import android.content.IIntentReceiver
+import android.content.IIntentSender
+import android.content.Intent
+import android.graphics.Bitmap
+import android.graphics.drawable.Icon
+import android.net.Uri
+import android.os.Binder
+import android.os.Bundle
+import android.os.IBinder
+import android.os.UserHandle
+import android.service.chooser.ChooserAction
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(AndroidJUnit4::class)
+class UriFiltersTest {
+
+    @Test
+    fun uri_ownedByCurrentUser_noUserId() {
+        val uri = Uri.parse("content://media/images/12345")
+        assertTrue("Uri without userId should always return true", uri.ownedByCurrentUser)
+    }
+
+    @Test
+    fun uri_ownedByCurrentUser_selfUserId() {
+        val uri = Uri.parse("content://${UserHandle.myUserId()}@media/images/12345")
+        assertTrue("Uri with own userId should return true", uri.ownedByCurrentUser)
+    }
+
+    @Test
+    fun uri_ownedByCurrentUser_otherUserId() {
+        val otherUserId = UserHandle.myUserId() + 10
+        val uri = Uri.parse("content://${otherUserId}@media/images/12345")
+        assertFalse("Uri with other userId should return false", uri.ownedByCurrentUser)
+    }
+
+    @Test
+    fun chooserAction_hasValidIcon_bitmap() =
+        smallBitmap().use {
+            val icon = Icon.createWithBitmap(it)
+            val action = actionWithIcon(icon)
+            assertTrue("No uri, assumed valid", hasValidIcon(action))
+        }
+
+    @Test
+    fun chooserAction_hasValidIcon_uri() {
+        val icon = Icon.createWithContentUri("content://provider/content/12345")
+        assertTrue("No userId in uri, uri is valid", hasValidIcon(actionWithIcon(icon)))
+    }
+    @Test
+    fun chooserAction_hasValidIcon_uri_unowned() {
+        val userId = UserHandle.myUserId() + 10
+        val icon = Icon.createWithContentUri("content://${userId}@provider/content/12345")
+        assertFalse("uri userId references a different user", hasValidIcon(actionWithIcon(icon)))
+    }
+
+    private fun smallBitmap() = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888)
+
+    private fun mockAction(): PendingIntent {
+        return PendingIntent(
+            object : IIntentSender {
+                override fun asBinder(): IBinder = Binder()
+                override fun send(
+                    code: Int,
+                    intent: Intent?,
+                    resolvedType: String?,
+                    whitelistToken: IBinder?,
+                    finishedReceiver: IIntentReceiver?,
+                    requiredPermission: String?,
+                    options: Bundle?
+                ) {
+                    /* empty */
+                }
+            }
+        )
+    }
+
+    private fun actionWithIcon(icon: Icon): ChooserAction {
+        return ChooserAction.Builder(icon, "", mockAction()).build()
+    }
+
+    /** Unconditionally recycles the [Bitmap] after running the given block */
+    private fun Bitmap.use(block: (Bitmap) -> Unit) =
+        try {
+            block(this)
+        } finally {
+            recycle()
+        }
+}