From 92a5b0d59f529d264a635a91cb800f5b921c197b Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Wed, 13 Aug 2014 15:39:50 -0700 Subject: [PATCH] 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) --- core/java/android/os/Parcel.java | 27 +++++++++++++++++--- core/java/android/util/ArrayMap.java | 38 ++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java index 94b961793e267..25e6aa56db3fd 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 val) { + /* package */ void writeArrayMapInternal(ArrayMap val) { if (val == null) { writeInt(-1); return; @@ -614,7 +614,7 @@ public final class Parcel { int startPos; for (int i=0; i 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 df1d4cd7047a6..8ea23ba12c5c8 100644 --- a/core/java/android/util/ArrayMap.java +++ b/core/java/android/util/ArrayMap.java @@ -493,6 +493,44 @@ public final class ArrayMap implements Map { mArray[index+1] = value; } + /** + * 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=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 array * @param array The array whose contents are to be retrieved.