Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 54db81c5 authored by Winson Chiu's avatar Winson Chiu Committed by William Loh
Browse files

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
parent 7923f0db
Loading
Loading
Loading
Loading
+18 −11
Original line number Diff line number Diff line
@@ -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);
+8 −0
Original line number Diff line number Diff line
@@ -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"
}
+49 −0
Original line number Diff line number Diff line
@@ -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;
@@ -1970,6 +1971,42 @@ public abstract class Uri implements Parcelable, Comparable<Uri> {
                || (allow != null && allow.indexOf(c) != NOT_FOUND);
    }

    /**
     * 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
@@ -1987,6 +2024,18 @@ public abstract class Uri implements Parcelable, Comparable<Uri> {
                s, false /* convertPlus */, StandardCharsets.UTF_8, false /* throwOnFailure */);
    }

    /**
     * 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.
     */
+10 −0
Original line number Diff line number Diff line
@@ -214,6 +214,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: [