Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Safer @hide Bundle.getParcelable() with explicit type

Browse Source 
Provider a safer API getParcelable() that takes a Class<?> parameter
just like the safer Parcel APIs introduced, so we check the type before
deserializing, preventing unexpected and potentially vulnerable code
being executed (technique used in the bugs).

Making it @hide since ASA requested this in T (more details on bugs)
and the urgent usage is inside the platform, we can flesh out a public
API for U.

Test: App code gpaste/6130483466338304 logs gpaste/5148052949041152
Bug: 213169612
Bug: 212804042
Bug: 212803946
Bug: 210885162

Change-Id: Ieebc044043e0776e71d35c1cc11be9299f972c45
This commit is contained in:
Bernardo Rufino
2022-02-09 09:53:13 +00:00
parent 14da91737a
commit 0206268a12
4 changed files with 77 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
core/java/android/app/ActivityOptions.java
View File

@@ -1121,7 +1121,7 @@ public class ActivityOptions extends ComponentOptions {
mPackageName = opts.getString(KEY_PACKAGE_NAME);
try {
mUsageTimeReport = opts.getParcelable(KEY_USAGE_TIME_REPORT);
mUsageTimeReport = opts.getParcelable(KEY_USAGE_TIME_REPORT, PendingIntent.class);
} catch (RuntimeException e) {
Slog.w(TAG, e);
}

52
core/java/android/os/BaseBundle.java
View File

@@ -31,7 +31,7 @@ import com.android.internal.util.IndentingPrintWriter;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Set;
import java.util.function.Supplier;
import java.util.function.Function;
/**
* A mapping from String keys to values of various types. In most cases, you
@@ -252,11 +252,10 @@ public class BaseBundle {
if (size == 0) {
return null;
}
Object o = getValueAt(0);
try {
return (String) o;
} catch (ClassCastException e) {
typeWarning("getPairValue()", o, "String", e);
return getValueAt(0, String.class);
} catch (ClassCastException | BadParcelableException e) {
typeWarning("getPairValue()", /* value */ null, "String", e);
return null;
}
}
@@ -309,7 +308,7 @@ public class BaseBundle {
}
for (int i = 0, n = mMap.size(); i < n; i++) {
// Triggers deserialization of i-th item, if needed
getValueAt(i);
getValueAt(i, /* clazz */ null);
}
}
}
@@ -324,8 +323,21 @@ public class BaseBundle {
* @hide
*/
final Object getValue(String key) {
return getValue(key, /* clazz */ null);
}
/**
* Returns the value for key {@code key} for expected return type {@param clazz} (or {@code
* null} for no type check).
*
* This call should always be made after {@link #unparcel()} or inside a lock after making sure
* {@code mMap} is not null.
*
* @hide
*/
final <T> T getValue(String key, @Nullable Class<T> clazz) {
int i = mMap.indexOfKey(key);
return (i >= 0) ? getValueAt(i) : null;
return (i >= 0) ? getValueAt(i, clazz) : null;
}
/**
@@ -336,11 +348,12 @@ public class BaseBundle {
*
* @hide
*/
final Object getValueAt(int i) {
@SuppressWarnings("unchecked")
final <T> T getValueAt(int i, @Nullable Class<T> clazz) {
Object object = mMap.valueAt(i);
if (object instanceof Supplier<?>) {
if (object instanceof Function<?, ?>) {
try {
object = ((Supplier<?>) object).get();
object = ((Function<Class<?>, ?>) object).apply(clazz);
} catch (BadParcelableException e) {
if (sShouldDefuse) {
Log.w(TAG, "Failed to parse item " + mMap.keyAt(i) + ", returning null.", e);
@@ -351,7 +364,7 @@ public class BaseBundle {
}
mMap.setValueAt(i, object);
}
return object;
return (clazz != null) ? clazz.cast(object) : (T) object;
}
private void initializeFromParcelLocked(@NonNull Parcel parcelledData, boolean recycleParcel,
@@ -528,7 +541,7 @@ public class BaseBundle {
} else {
// Following semantic above of failing in case we get a serialized value vs a
// deserialized one, we'll compare the map. If a certain element hasn't been
// deserialized yet, it's a Supplier (or more specifically a LazyValue, but let's
// deserialized yet, it's a function object (or more specifically a LazyValue, but let's
// pretend we don't know that here :P), we'll use that element's equality comparison as
// map naturally does. That will takes care of comparing the payload if needed (see
// Parcel.readLazyValue() for details).
@@ -982,15 +995,19 @@ public class BaseBundle {
}
// Log a message if the value was non-null but not of the expected type
void typeWarning(String key, Object value, String className,
Object defaultValue, ClassCastException e) {
void typeWarning(String key, @Nullable Object value, String className,
Object defaultValue, RuntimeException e) {
StringBuilder sb = new StringBuilder();
sb.append("Key ");
sb.append(key);
sb.append(" expected ");
sb.append(className);
sb.append(" but value was a ");
sb.append(value.getClass().getName());
if (value != null) {
sb.append(" but value was a ");
sb.append(value.getClass().getName());
} else {
sb.append(" but value was of a different type ");
}
sb.append(". The default value ");
sb.append(defaultValue);
sb.append(" was returned.");
@@ -998,8 +1015,7 @@ public class BaseBundle {
Log.w(TAG, "Attempt to cast generated internal exception:", e);
}
void typeWarning(String key, Object value, String className,
ClassCastException e) {
void typeWarning(String key, @Nullable Object value, String className, RuntimeException e) {
typeWarning(key, value, className, "<null>", e);
}

30
core/java/android/os/Bundle.java
View File

@@ -16,6 +16,9 @@
package android.os;
import static java.util.Objects.requireNonNull;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.compat.annotation.UnsupportedAppUsage;
import android.util.ArrayMap;
@@ -912,6 +915,33 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
}
}
/**
* Returns the value associated with the given key, or {@code null} if
* no mapping of the desired type exists for the given key or a {@code null}
* value is explicitly associated with the key.
*
* <p><b>Note: </b> if the expected value is not a class provided by the Android platform,
* you must call {@link #setClassLoader(ClassLoader)} with the proper {@link ClassLoader} first.
* Otherwise, this method might throw an exception or return {@code null}.
*
* @param key a String, or {@code null}
* @param clazz The type of the object expected or {@code null} for performing no checks.
* @return a Parcelable value, or {@code null}
*
* @hide
*/
@SuppressWarnings("unchecked")
@Nullable
public <T> T getParcelable(@Nullable String key, @NonNull Class<T> clazz) {
unparcel();
try {
return getValue(key, requireNonNull(clazz));
} catch (ClassCastException | BadParcelableException e) {
typeWarning(key, /* value */ null, "Parcelable", e);
return null;
}
}
/**
* Returns the value associated with the given key, or {@code null} if
* no mapping of the desired type exists for the given key or a null

23
core/java/android/os/Parcel.java
View File

@@ -4319,18 +4319,19 @@ public final class Parcel {
}
/**
* This will return a {@link Supplier} for length-prefixed types that deserializes the object
* when {@link Supplier#get()} is called, for other types it will return the object itself.
* This will return a {@link Function} for length-prefixed types that deserializes the object
* when {@link Function#apply} is called with the expected class of the return object (or {@code
* null} for no type check), for other types it will return the object itself.
*
* <p>After calling {@link Supplier#get()} the parcel cursor will not change. Note that you
* shouldn't recycle the parcel, not at least until all objects have been retrieved. No
* <p>After calling {@link Function#apply(Object)} the parcel cursor will not change. Note that
* you shouldn't recycle the parcel, not at least until all objects have been retrieved. No
* synchronization attempts are made.
*
* </p>The supplier returned implements {@link #equals(Object)} and {@link #hashCode()}. Two
* suppliers are equal if either of the following is true:
* </p>The function returned implements {@link #equals(Object)} and {@link #hashCode()}. Two
* function objects are equal if either of the following is true:
* <ul>
* <li>{@link Supplier#get()} has been called on both and both objects returned are equal.
* <li>{@link Supplier#get()} hasn't been called on either one and everything below is true:
* <li>{@link Function#apply} has been called on both and both objects returned are equal.
* <li>{@link Function#apply} hasn't been called on either one and everything below is true:
* <ul>
* <li>The {@code loader} parameters used to retrieve each are equal.
* <li>They both have the same type.
@@ -4357,7 +4358,7 @@ public final class Parcel {
}
private static final class LazyValue implements Supplier<Object> {
private static final class LazyValue implements Function<Class<?>, Object> {
/**
* | 4B | 4B |
* mSource = Parcel{... | type | length | object | ...}
@@ -4389,7 +4390,7 @@ public final class Parcel {
}
@Override
public Object get() {
public Object apply(@Nullable Class<?> clazz) {
Parcel source = mSource;
if (source != null) {
synchronized (source) {
@@ -4398,7 +4399,7 @@ public final class Parcel {
int restore = source.dataPosition();
try {
source.setDataPosition(mPosition);
mObject = source.readValue(mLoader);
mObject = source.readValue(mLoader, clazz);
} finally {
source.setDataPosition(restore);
}