Fix issue #16794553: Duplicate ArrayMap entries in Bundle...

...can lead to launching of un-exported activities

We now validate the array map after unparcelling to make sure there
are no duplicate keys.

And to make up for the performance overhead this introduces, I switched
the parcelling/unparcelling code to write keys as explicit string
objects rather than generic values.  There was no reason to use generic
values since the write method itself only accepts an array map with
String keys.

Change-Id: I57bda9eb79ceaaa9c1b94ad49d9e462b52102149
(cherry picked from commit cee4cdad307071a8ff9a3e2189bf9e6fcc417222)

2 files changed, 61 insertions, 4 deletions

diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java
index 94b9617..25e6aa5 100644
--- a/core/java/android/os/Parcel.java
+++ b/core/java/android/os/Parcel.java
@@ -599,7 +599,7 @@ public final class Parcel {
      * Flatten an ArrayMap into the parcel at the current dataPosition(),
      * growing dataCapacity() if needed.  The Map keys must be String objects.
      */
-    /* package */ void writeArrayMapInternal(ArrayMap<String,Object> val) {
+    /* package */ void writeArrayMapInternal(ArrayMap<String, Object> val) {
         if (val == null) {
             writeInt(-1);
             return;
@@ -614,7 +614,7 @@ public final class Parcel {
         int startPos;
         for (int i=0; i<N; i++) {
             if (DEBUG_ARRAY_MAP) startPos = dataPosition();
-            writeValue(val.keyAt(i));
+            writeString(val.keyAt(i));
             writeValue(val.valueAt(i));
             if (DEBUG_ARRAY_MAP) Log.d(TAG, "  Write #" + i + " "
                     + (dataPosition()-startPos) + " bytes: key=0x"
@@ -624,6 +624,13 @@ public final class Parcel {
     }
 
     /**
+     * @hide For testing only.
+     */
+    public void writeArrayMap(ArrayMap<String, Object> val) {
+        writeArrayMapInternal(val);
+    }
+
+    /**
      * Flatten a Bundle into the parcel at the current dataPosition(),
      * growing dataCapacity() if needed.
      */
@@ -2310,7 +2317,7 @@ public final class Parcel {
         int startPos;
         while (N > 0) {
             if (DEBUG_ARRAY_MAP) startPos = dataPosition();
-            Object key = readValue(loader);
+            String key = readString();
             Object value = readValue(loader);
             if (DEBUG_ARRAY_MAP) Log.d(TAG, "  Read #" + (N-1) + " "
                     + (dataPosition()-startPos) + " bytes: key=0x"
@@ -2318,6 +2325,7 @@ public final class Parcel {
             outVal.append(key, value);
             N--;
         }
+        outVal.validate();
     }
 
     /* package */ void readArrayMapSafelyInternal(ArrayMap outVal, int N,
@@ -2328,7 +2336,7 @@ public final class Parcel {
             Log.d(TAG, "Reading safely " + N + " ArrayMap entries", here);
         }
         while (N > 0) {
-            Object key = readValue(loader);
+            String key = readString();
             if (DEBUG_ARRAY_MAP) Log.d(TAG, "  Read safe #" + (N-1) + ": key=0x"
                     + (key != null ? key.hashCode() : 0) + " " + key);
             Object value = readValue(loader);
@@ -2337,6 +2345,17 @@ public final class Parcel {
         }
     }
 
+    /**
+     * @hide For testing only.
+     */
+    public void readArrayMap(ArrayMap outVal, ClassLoader loader) {
+        final int N = readInt();
+        if (N < 0) {
+            return;
+        }
+        readArrayMapInternal(outVal, N, loader);
+    }
+
     private void readListInternal(List outVal, int N,
         ClassLoader loader) {
         while (N > 0) {
diff --git a/core/java/android/util/ArrayMap.java b/core/java/android/util/ArrayMap.java
index df1d4cd..8ea23ba 100644
--- a/core/java/android/util/ArrayMap.java
+++ b/core/java/android/util/ArrayMap.java
@@ -494,6 +494,44 @@ public final class ArrayMap<K, V> implements Map<K, V> {
     }
 
     /**
+     * The use of the {@link #append} function can result in invalid array maps, in particular
+     * an array map where the same key appears multiple times.  This function verifies that
+     * the array map is valid, throwing IllegalArgumentException if a problem is found.  The
+     * main use for this method is validating an array map after unpacking from an IPC, to
+     * protect against malicious callers.
+     * @hide
+     */
+    public void validate() {
+        final int N = mSize;
+        if (N <= 1) {
+            // There can't be dups.
+            return;
+        }
+        int basehash = mHashes[0];
+        int basei = 0;
+        for (int i=1; i<N; i++) {
+            int hash = mHashes[i];
+            if (hash != basehash) {
+                basehash = hash;
+                basei = i;
+                continue;
+            }
+            // We are in a run of entries with the same hash code.  Go backwards through
+            // the array to see if any keys are the same.
+            final Object cur = mArray[i<<1];
+            for (int j=i-1; j>=basei; j--) {
+                final Object prev = mArray[j<<1];
+                if (cur == prev) {
+                    throw new IllegalArgumentException("Duplicate key in ArrayMap: " + cur);
+                }
+                if (cur != null && prev != null && cur.equals(prev)) {
+                    throw new IllegalArgumentException("Duplicate key in ArrayMap: " + cur);
+                }
+            }
+        }
+    }
+
+    /**
      * Perform a {@link #put(Object, Object)} of all key/value pairs in <var>array</var>
      * @param array The array whose contents are to be retrieved.
      */