From ec4b60c1ad5f6ba683f3e8e018f1f72ed3374310 Mon Sep 17 00:00:00 2001 From: Hao Ke Date: Mon, 25 Oct 2021 20:07:32 +0000 Subject: Adding typed Parcel read APIs for Serializable. Added typed read API of `readSerializable`, that takes an extra Class parameter to check that the class written on the wire is the same or a descendant from the one provided as argument. Doing so could enhance the security of Parcel deserialization, as it would prevent unexpected types of objects being deserialized. More details can be found at go/safer-parcel. Test: atest -d android.os.cts.ParcelTest Bug: 195622897 Change-Id: I94e48ac7fe8f5d3ba4c54100cb76ce3e4aacdd09 --- core/api/current.txt | 1 + core/java/android/os/Parcel.java | 84 ++++++++++++++++++++++++++++++---------- 2 files changed, 65 insertions(+), 20 deletions(-) diff --git a/core/api/current.txt b/core/api/current.txt index 0eba9d75fdfc..717be996e853 100644 --- a/core/api/current.txt +++ b/core/api/current.txt @@ -31466,6 +31466,7 @@ package android.os { method @Nullable public android.os.PersistableBundle readPersistableBundle(); method @Nullable public android.os.PersistableBundle readPersistableBundle(@Nullable ClassLoader); method @Nullable public java.io.Serializable readSerializable(); + method @Nullable public T readSerializable(@Nullable ClassLoader, @NonNull Class); method @NonNull public android.util.Size readSize(); method @NonNull public android.util.SizeF readSizeF(); method @Nullable public android.util.SparseArray readSparseArray(@Nullable ClassLoader); diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java index 6be86da9a925..79a65cba331f 100644 --- a/core/java/android/os/Parcel.java +++ b/core/java/android/os/Parcel.java @@ -2440,9 +2440,9 @@ public final class Parcel { writeByteArray(baos.toByteArray()); } catch (IOException ioe) { - throw new RuntimeException("Parcelable encountered " + - "IOException writing serializable object (name = " + name + - ")", ioe); + throw new BadParcelableException("Parcelable encountered " + + "IOException writing serializable object (name = " + + name + ")", ioe); } } @@ -3794,7 +3794,7 @@ public final class Parcel { break; case VAL_SERIALIZABLE: - object = readSerializable(loader); + object = readSerializableInternal(loader, clazz); break; case VAL_PARCELABLEARRAY: @@ -3910,7 +3910,6 @@ public final class Parcel { } /** - * * @param clazz The type of the parcelable expected or {@code null} for performing no checks. */ @SuppressWarnings("unchecked") @@ -4113,12 +4112,37 @@ public final class Parcel { * wasn't found in the parcel. */ @Nullable - public final Serializable readSerializable() { - return readSerializable(null); + public Serializable readSerializable() { + return readSerializableInternal(/* loader */ null, /* clazz */ null); } + /** + * Same as {@link #readSerializable()} but accepts {@code loader} parameter + * as the primary classLoader for resolving the Serializable class; and {@code clazz} parameter + * as the required type. + * + * @throws BadParcelableException Throws BadParcelableException if the item to be deserialized + * is not an instance of that class or any of its children class or there there was an error + * deserializing the object. + */ @Nullable - private final Serializable readSerializable(@Nullable final ClassLoader loader) { + public T readSerializable(@Nullable ClassLoader loader, + @NonNull Class clazz) { + Objects.requireNonNull(clazz); + return readSerializableInternal(loader, clazz); + } + + /** + * @param clazz The type of the serializable expected or {@code null} for performing no checks + */ + @Nullable + private T readSerializableInternal(@Nullable final ClassLoader loader, + @Nullable Class clazz) { + if (clazz != null && !Serializable.class.isAssignableFrom(clazz)) { + throw new BadParcelableException("About to unparcel a serializable object " + + " but class required " + clazz.getName() + " is not Serializable"); + } + String name = readString(); if (name == null) { // For some reason we were unable to read the name of the Serializable (either there @@ -4127,9 +4151,20 @@ public final class Parcel { return null; } - byte[] serializedData = createByteArray(); - ByteArrayInputStream bais = new ByteArrayInputStream(serializedData); try { + if (clazz != null && loader != null) { + // If custom classloader is provided, resolve the type of serializable using the + // name, then check the type before deserialization. As in this case we can resolve + // the class the same way as ObjectInputStream, using the provided classloader. + Class cl = Class.forName(name, false, loader); + if (!clazz.isAssignableFrom(cl)) { + throw new BadParcelableException("Serializable object " + + cl.getName() + " is not a subclass of required class " + + clazz.getName() + " provided in the parameter"); + } + } + byte[] serializedData = createByteArray(); + ByteArrayInputStream bais = new ByteArrayInputStream(serializedData); ObjectInputStream ois = new ObjectInputStream(bais) { @Override protected Class resolveClass(ObjectStreamClass osClass) @@ -4137,22 +4172,31 @@ public final class Parcel { // try the custom classloader if provided if (loader != null) { Class c = Class.forName(osClass.getName(), false, loader); - if (c != null) { - return c; - } + return Objects.requireNonNull(c); } return super.resolveClass(osClass); } }; - return (Serializable) ois.readObject(); + T object = (T) ois.readObject(); + if (clazz != null && loader == null) { + // If custom classloader is not provided, check the type of the serializable using + // the deserialized object, as we cannot resolve the class the same way as + // ObjectInputStream. + if (!clazz.isAssignableFrom(object.getClass())) { + throw new BadParcelableException("Serializable object " + + object.getClass().getName() + " is not a subclass of required class " + + clazz.getName() + " provided in the parameter"); + } + } + return object; } catch (IOException ioe) { - throw new RuntimeException("Parcelable encountered " + - "IOException reading a Serializable object (name = " + name + - ")", ioe); + throw new BadParcelableException("Parcelable encountered " + + "IOException reading a Serializable object (name = " + + name + ")", ioe); } catch (ClassNotFoundException cnfe) { - throw new RuntimeException("Parcelable encountered " + - "ClassNotFoundException reading a Serializable object (name = " - + name + ")", cnfe); + throw new BadParcelableException("Parcelable encountered " + + "ClassNotFoundException reading a Serializable object (name = " + + name + ")", cnfe); } } -- cgit v1.2.3-59-g8ed1b