Encode values when serializing android-app Intent

And decodes them when reconstructing the Intent.

Also adds methods to Uri to check whether a string is already
encoded (whether it does not contain any non-allowed characters)
which are used to enforce that values are encoded when serializing.

This does not solve the problem for generic URIs that were not
serialized using the framework APIs, as it's not possible to tell
whether or not a particular string containing invalid characters
should be encoded or not.

For example, in a query string, `key=value` might accidentally encode
the `=`, which might break some usages of Uri.Part.

This is a high risk that this will break the parsing of arbitrary
URIs which do not conform to the encoding structure the framework
APIs expect, but would otherwise work before this change.

Bug: 281848623
Bug: 281849097
Bug: 281849163

Test: atest android.content.cts.IntentEncodingParameterizedTest

Change-Id: I6e3e93247a8ac02e661d267976c7b6e7093a47c1

4 files changed, 85 insertions, 11 deletions

diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java
index ee1d117bf71c..d5eee63fee12 100644
--- a/core/java/android/content/Intent.java
+++ b/core/java/android/content/Intent.java
@@ -8099,7 +8099,7 @@ public class Intent implements Parcelable, Cloneable {
                         int end = data.indexOf('/', 14);
                         if (end < 0) {
                             // All we have is a package name.
-                            intent.mPackage = data.substring(14);
+                            intent.mPackage = Uri.decodeIfNeeded(data.substring(14));
                             if (!explicitAction) {
                                 intent.setAction(ACTION_MAIN);
                             }
@@ -8107,21 +8107,22 @@ public class Intent implements Parcelable, Cloneable {
                         } else {
                             // Target the Intent at the given package name always.
                             String authority = null;
-                            intent.mPackage = data.substring(14, end);
+                            intent.mPackage = Uri.decodeIfNeeded(data.substring(14, end));
                             int newEnd;
                             if ((end+1) < data.length()) {
                                 if ((newEnd=data.indexOf('/', end+1)) >= 0) {
                                     // Found a scheme, remember it.
-                                    scheme = data.substring(end+1, newEnd);
+                                    scheme = Uri.decodeIfNeeded(data.substring(end + 1, newEnd));
                                     end = newEnd;
                                     if (end < data.length() && (newEnd=data.indexOf('/', end+1)) >= 0) {
                                         // Found a authority, remember it.
-                                        authority = data.substring(end+1, newEnd);
+                                        authority = Uri.decodeIfNeeded(
+                                                data.substring(end + 1, newEnd));
                                         end = newEnd;
                                     }
                                 } else {
                                     // All we have is a scheme.
-                                    scheme = data.substring(end+1);
+                                    scheme = Uri.decodeIfNeeded(data.substring(end + 1));
                                 }
                             }
                             if (scheme == null) {
@@ -11762,27 +11763,33 @@ public class Intent implements Parcelable, Cloneable {
                         + this);
             }
             uri.append("android-app://");
-            uri.append(mPackage);
+            uri.append(Uri.encode(mPackage));
             String scheme = null;
             if (mData != null) {
-                scheme = mData.getScheme();
+                // All values here must be wrapped with Uri#encodeIfNotEncoded because it is
+                // possible to exploit the Uri API to return a raw unencoded value, which will
+                // not deserialize properly and may cause the resulting Intent to be transformed
+                // to a malicious value.
+                scheme = Uri.encodeIfNotEncoded(mData.getScheme(), null);
                 if (scheme != null) {
                     uri.append('/');
                     uri.append(scheme);
-                    String authority = mData.getEncodedAuthority();
+                    String authority = Uri.encodeIfNotEncoded(mData.getEncodedAuthority(), null);
                     if (authority != null) {
                         uri.append('/');
                         uri.append(authority);
-                        String path = mData.getEncodedPath();
+
+                        // Multiple path segments are allowed, don't encode the path / separator
+                        String path = Uri.encodeIfNotEncoded(mData.getEncodedPath(), "/");
                         if (path != null) {
                             uri.append(path);
                         }
-                        String queryParams = mData.getEncodedQuery();
+                        String queryParams = Uri.encodeIfNotEncoded(mData.getEncodedQuery(), null);
                         if (queryParams != null) {
                             uri.append('?');
                             uri.append(queryParams);
                         }
-                        String fragment = mData.getEncodedFragment();
+                        String fragment = Uri.encodeIfNotEncoded(mData.getEncodedFragment(), null);
                         if (fragment != null) {
                             uri.append('#');
                             uri.append(fragment);
diff --git a/core/java/android/content/pm/flags.aconfig b/core/java/android/content/pm/flags.aconfig
index 0b60977e48c7..a2cd3e153b3e 100644
--- a/core/java/android/content/pm/flags.aconfig
+++ b/core/java/android/content/pm/flags.aconfig
@@ -138,3 +138,11 @@ flag {
     description: "Add a new FGS type for media processing use cases."
     bug: "317788011"
 }
+
+flag {
+    name: "encode_app_intent"
+    namespace: "package_manager_service"
+    description: "Feature flag to encode app intent."
+    bug: "281848623"
+}
+
diff --git a/core/java/android/net/Uri.java b/core/java/android/net/Uri.java
index 70de477e9e2e..05a3e182135c 100644
--- a/core/java/android/net/Uri.java
+++ b/core/java/android/net/Uri.java
@@ -21,6 +21,7 @@ import android.annotation.Nullable;
 import android.annotation.SystemApi;
 import android.compat.annotation.UnsupportedAppUsage;
 import android.content.Intent;
+import android.content.pm.Flags;
 import android.os.Environment;
 import android.os.Parcel;
 import android.os.Parcelable;
@@ -1971,6 +1972,42 @@ public abstract class Uri implements Parcelable, Comparable<Uri> {
     }
 
     /**
+     * Encodes a value it wasn't already encoded.
+     *
+     * @param value string to encode
+     * @param allow characters to allow
+     * @return encoded value
+     * @hide
+     */
+    public static String encodeIfNotEncoded(@Nullable String value, @Nullable String allow) {
+        if (value == null) return null;
+        if (!Flags.encodeAppIntent() || isEncoded(value, allow)) return value;
+        return encode(value, allow);
+    }
+
+    /**
+     * Returns true if the given string is already encoded to safe characters.
+     *
+     * @param value string to check
+     * @param allow characters to allow
+     * @return true if the string is already encoded or false if it should be encoded
+     */
+    private static boolean isEncoded(@Nullable String value, @Nullable String allow) {
+        if (value == null) return true;
+        for (int index = 0; index < value.length(); index++) {
+            char c = value.charAt(index);
+
+            // Allow % because that's the prefix for an encoded character. This method will fail
+            // for decoded strings whose onlyinvalid character is %, but it's assumed that % alone
+            // cannot cause malicious behavior in the framework.
+            if (!isAllowed(c, allow) && c != '%') {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /**
      * Decodes '%'-escaped octets in the given string using the UTF-8 scheme.
      * Replaces invalid octets with the unicode replacement character
      * ("\\uFFFD").
@@ -1988,6 +2025,18 @@ public abstract class Uri implements Parcelable, Comparable<Uri> {
     }
 
     /**
+     * Decodes a string if it was encoded, indicated by containing a %.
+     * @param value encoded string to decode
+     * @return decoded value
+     * @hide
+     */
+    public static String decodeIfNeeded(@Nullable String value) {
+        if (value == null) return null;
+        if (Flags.encodeAppIntent() && value.contains("%")) return decode(value);
+        return value;
+    }
+
+    /**
      * Support for part implementations.
      */
     static abstract class AbstractPart {
diff --git a/services/tests/servicestests/Android.bp b/services/tests/servicestests/Android.bp
index 9b8419021c5c..00450267ee79 100644
--- a/services/tests/servicestests/Android.bp
+++ b/services/tests/servicestests/Android.bp
@@ -215,6 +215,16 @@ java_library {
 }
 
 java_library {
+    name: "mockito-test-utils",
+    srcs: [
+        "utils-mockito/**/*.kt",
+    ],
+    static_libs: [
+        "mockito-target-minus-junit4",
+    ],
+}
+
+java_library {
     name: "servicestests-utils-mockito-extended",
     srcs: [
         "utils/**/*.java",