Skip to content

Commit f415bf5

Browse files
Dianne Hackbornandi34
authored andcommitted
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
1 parent f702e7c commit f415bf5

File tree

2 files changed

+61
-4
lines changed

2 files changed

+61
-4
lines changed

core/java/android/os/Parcel.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ public final void writeMap(Map val) {
599599
* Flatten an ArrayMap into the parcel at the current dataPosition(),
600600
* growing dataCapacity() if needed. The Map keys must be String objects.
601601
*/
602-
/* package */ void writeArrayMapInternal(ArrayMap<String,Object> val) {
602+
/* package */ void writeArrayMapInternal(ArrayMap<String, Object> val) {
603603
if (val == null) {
604604
writeInt(-1);
605605
return;
@@ -614,7 +614,7 @@ public final void writeMap(Map val) {
614614
int startPos;
615615
for (int i=0; i<N; i++) {
616616
if (DEBUG_ARRAY_MAP) startPos = dataPosition();
617-
writeValue(val.keyAt(i));
617+
writeString(val.keyAt(i));
618618
writeValue(val.valueAt(i));
619619
if (DEBUG_ARRAY_MAP) Log.d(TAG, " Write #" + i + " "
620620
+ (dataPosition()-startPos) + " bytes: key=0x"
@@ -623,6 +623,13 @@ public final void writeMap(Map val) {
623623
}
624624
}
625625

626+
/**
627+
* @hide For testing only.
628+
*/
629+
public void writeArrayMap(ArrayMap<String, Object> val) {
630+
writeArrayMapInternal(val);
631+
}
632+
626633
/**
627634
* Flatten a Bundle into the parcel at the current dataPosition(),
628635
* growing dataCapacity() if needed.
@@ -2310,14 +2317,15 @@ protected void finalize() throws Throwable {
23102317
int startPos;
23112318
while (N > 0) {
23122319
if (DEBUG_ARRAY_MAP) startPos = dataPosition();
2313-
Object key = readValue(loader);
2320+
String key = readString();
23142321
Object value = readValue(loader);
23152322
if (DEBUG_ARRAY_MAP) Log.d(TAG, " Read #" + (N-1) + " "
23162323
+ (dataPosition()-startPos) + " bytes: key=0x"
23172324
+ Integer.toHexString((key != null ? key.hashCode() : 0)) + " " + key);
23182325
outVal.append(key, value);
23192326
N--;
23202327
}
2328+
outVal.validate();
23212329
}
23222330

23232331
/* package */ void readArrayMapSafelyInternal(ArrayMap outVal, int N,
@@ -2328,7 +2336,7 @@ protected void finalize() throws Throwable {
23282336
Log.d(TAG, "Reading safely " + N + " ArrayMap entries", here);
23292337
}
23302338
while (N > 0) {
2331-
Object key = readValue(loader);
2339+
String key = readString();
23322340
if (DEBUG_ARRAY_MAP) Log.d(TAG, " Read safe #" + (N-1) + ": key=0x"
23332341
+ (key != null ? key.hashCode() : 0) + " " + key);
23342342
Object value = readValue(loader);
@@ -2337,6 +2345,17 @@ protected void finalize() throws Throwable {
23372345
}
23382346
}
23392347

2348+
/**
2349+
* @hide For testing only.
2350+
*/
2351+
public void readArrayMap(ArrayMap outVal, ClassLoader loader) {
2352+
final int N = readInt();
2353+
if (N < 0) {
2354+
return;
2355+
}
2356+
readArrayMapInternal(outVal, N, loader);
2357+
}
2358+
23402359
private void readListInternal(List outVal, int N,
23412360
ClassLoader loader) {
23422361
while (N > 0) {

core/java/android/util/ArrayMap.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,44 @@ public void append(K key, V value) {
493493
mArray[index+1] = value;
494494
}
495495

496+
/**
497+
* The use of the {@link #append} function can result in invalid array maps, in particular
498+
* an array map where the same key appears multiple times. This function verifies that
499+
* the array map is valid, throwing IllegalArgumentException if a problem is found. The
500+
* main use for this method is validating an array map after unpacking from an IPC, to
501+
* protect against malicious callers.
502+
* @hide
503+
*/
504+
public void validate() {
505+
final int N = mSize;
506+
if (N <= 1) {
507+
// There can't be dups.
508+
return;
509+
}
510+
int basehash = mHashes[0];
511+
int basei = 0;
512+
for (int i=1; i<N; i++) {
513+
int hash = mHashes[i];
514+
if (hash != basehash) {
515+
basehash = hash;
516+
basei = i;
517+
continue;
518+
}
519+
// We are in a run of entries with the same hash code. Go backwards through
520+
// the array to see if any keys are the same.
521+
final Object cur = mArray[i<<1];
522+
for (int j=i-1; j>=basei; j--) {
523+
final Object prev = mArray[j<<1];
524+
if (cur == prev) {
525+
throw new IllegalArgumentException("Duplicate key in ArrayMap: " + cur);
526+
}
527+
if (cur != null && prev != null && cur.equals(prev)) {
528+
throw new IllegalArgumentException("Duplicate key in ArrayMap: " + cur);
529+
}
530+
}
531+
}
532+
}
533+
496534
/**
497535
* Perform a {@link #put(Object, Object)} of all key/value pairs in <var>array</var>
498536
* @param array The array whose contents are to be retrieved.

0 commit comments

Comments
 (0)