Merge "Fix ParceledListSlice to enforce the same concrete types among its elements." into lmp-mr1-dev

This commit is contained in:
Adam Lesinski
2014-11-13 00:32:59 +00:00
committed by Android (Google) Code Review
2 changed files with 306 additions and 4 deletions

View File

@ -30,6 +30,12 @@ import java.util.List;
* Transfer a large list of Parcelable objects across an IPC. Splits into
* multiple transactions if needed.
*
* Caveat: for efficiency and security, all elements must be the same concrete type.
* In order to avoid writing the class name of each object, we must ensure that
* each object is the same type, or else unparceling then reparceling the data may yield
* a different result if the class name encoded in the Parcelable is a Base type.
* See b/17671747.
*
* @hide
*/
public class ParceledListSlice<T extends Parcelable> implements Parcelable {
@ -56,13 +62,25 @@ public class ParceledListSlice<T extends Parcelable> implements Parcelable {
if (N <= 0) {
return;
}
Parcelable.Creator<T> creator = p.readParcelableCreator(loader);
Class<?> listElementClass = null;
int i = 0;
while (i < N) {
if (p.readInt() == 0) {
break;
}
mList.add(p.readCreator(creator, loader));
final T parcelable = p.readCreator(creator, loader);
if (listElementClass == null) {
listElementClass = parcelable.getClass();
} else {
verifySameType(listElementClass, parcelable.getClass());
}
mList.add(parcelable);
if (DEBUG) Log.d(TAG, "Read inline #" + i + ": " + mList.get(mList.size()-1));
i++;
}
@ -82,7 +100,11 @@ public class ParceledListSlice<T extends Parcelable> implements Parcelable {
return;
}
while (i < N && reply.readInt() != 0) {
mList.add(reply.readCreator(creator, loader));
final T parcelable = reply.readCreator(creator, loader);
verifySameType(listElementClass, parcelable.getClass());
mList.add(parcelable);
if (DEBUG) Log.d(TAG, "Read extra #" + i + ": " + mList.get(mList.size()-1));
i++;
}
@ -91,6 +113,14 @@ public class ParceledListSlice<T extends Parcelable> implements Parcelable {
}
}
private static void verifySameType(final Class<?> expected, final Class<?> actual) {
if (!actual.equals(expected)) {
throw new IllegalArgumentException("Can't unparcel type "
+ actual.getName() + " in list of type "
+ expected.getName());
}
}
public List<T> getList() {
return mList;
}
@ -116,11 +146,16 @@ public class ParceledListSlice<T extends Parcelable> implements Parcelable {
dest.writeInt(N);
if (DEBUG) Log.d(TAG, "Writing " + N + " items");
if (N > 0) {
final Class<?> listElementClass = mList.get(0).getClass();
dest.writeParcelableCreator(mList.get(0));
int i = 0;
while (i < N && dest.dataSize() < MAX_FIRST_IPC_SIZE) {
dest.writeInt(1);
mList.get(i).writeToParcel(dest, callFlags);
final T parcelable = mList.get(i);
verifySameType(listElementClass, parcelable.getClass());
parcelable.writeToParcel(dest, callFlags);
if (DEBUG) Log.d(TAG, "Wrote inline #" + i + ": " + mList.get(i));
i++;
}
@ -137,7 +172,11 @@ public class ParceledListSlice<T extends Parcelable> implements Parcelable {
if (DEBUG) Log.d(TAG, "Writing more @" + i + " of " + N);
while (i < N && reply.dataSize() < MAX_IPC_SIZE) {
reply.writeInt(1);
mList.get(i).writeToParcel(reply, callFlags);
final T parcelable = mList.get(i);
verifySameType(listElementClass, parcelable.getClass());
parcelable.writeToParcel(reply, callFlags);
if (DEBUG) Log.d(TAG, "Wrote extra #" + i + ": " + mList.get(i));
i++;
}

View File

@ -0,0 +1,263 @@
package android.content.pm;
import android.os.Parcel;
import android.os.Parcelable;
import junit.framework.TestCase;
import java.util.ArrayList;
import java.util.List;
public class ParceledListSliceTest extends TestCase {
public void testSmallList() throws Exception {
final int objectCount = 100;
List<SmallObject> list = new ArrayList<>();
for (int i = 0; i < objectCount; i++) {
list.add(new SmallObject(i * 2, (i * 2) + 1));
}
ParceledListSlice<SmallObject> slice;
Parcel parcel = Parcel.obtain();
try {
parcel.writeParcelable(new ParceledListSlice<>(list), 0);
parcel.setDataPosition(0);
slice = parcel.readParcelable(getClass().getClassLoader());
} finally {
parcel.recycle();
}
assertNotNull(slice);
assertNotNull(slice.getList());
assertEquals(objectCount, slice.getList().size());
for (int i = 0; i < objectCount; i++) {
assertEquals(i * 2, slice.getList().get(i).mFieldA);
assertEquals((i * 2) + 1, slice.getList().get(i).mFieldB);
}
}
private static int measureLargeObject() {
Parcel p = Parcel.obtain();
try {
new LargeObject(0, 0, 0, 0, 0).writeToParcel(p, 0);
return p.dataPosition();
} finally {
p.recycle();
}
}
/**
* Test that when the list is large, the data is successfully parceled
* and unparceled (the implementation will send pieces of the list in
* separate round-trips to avoid the IPC limit).
*/
public void testLargeList() throws Exception {
final int thresholdBytes = 256 * 1024;
final int objectCount = thresholdBytes / measureLargeObject();
List<LargeObject> list = new ArrayList<>();
for (int i = 0; i < objectCount; i++) {
list.add(new LargeObject(
i * 5,
(i * 5) + 1,
(i * 5) + 2,
(i * 5) + 3,
(i * 5) + 4
));
}
ParceledListSlice<LargeObject> slice;
Parcel parcel = Parcel.obtain();
try {
parcel.writeParcelable(new ParceledListSlice<>(list), 0);
parcel.setDataPosition(0);
slice = parcel.readParcelable(getClass().getClassLoader());
} finally {
parcel.recycle();
}
assertNotNull(slice);
assertNotNull(slice.getList());
assertEquals(objectCount, slice.getList().size());
for (int i = 0; i < objectCount; i++) {
assertEquals(i * 5, slice.getList().get(i).mFieldA);
assertEquals((i * 5) + 1, slice.getList().get(i).mFieldB);
assertEquals((i * 5) + 2, slice.getList().get(i).mFieldC);
assertEquals((i * 5) + 3, slice.getList().get(i).mFieldD);
assertEquals((i * 5) + 4, slice.getList().get(i).mFieldE);
}
}
/**
* Test that only homogeneous elements may be unparceled.
*/
public void testHomogeneousElements() throws Exception {
List<BaseObject> list = new ArrayList<>();
list.add(new LargeObject(0, 1, 2, 3, 4));
list.add(new SmallObject(5, 6));
list.add(new SmallObject(7, 8));
Parcel parcel = Parcel.obtain();
try {
writeEvilParceledListSlice(parcel, list);
parcel.setDataPosition(0);
try {
ParceledListSlice.CREATOR.createFromParcel(parcel, getClass().getClassLoader());
assertTrue("Unparceled heterogeneous ParceledListSlice", false);
} catch (IllegalArgumentException e) {
// Success, we're not allowed to process heterogeneous
// elements in a ParceledListSlice.
}
} finally {
parcel.recycle();
}
}
/**
* Write a ParcelableListSlice that uses the BaseObject base class as the Creator.
* This is dangerous, as it may affect how the data is unparceled, then later parceled
* by the system, leading to a self-modifying data security vulnerability.
*/
private static <T extends BaseObject> void writeEvilParceledListSlice(Parcel dest, List<T> list) {
final int listCount = list.size();
// Number of items.
dest.writeInt(listCount);
// The type/creator to use when unparceling. Here we use the base class
// to simulate an attack on ParceledListSlice.
dest.writeString(BaseObject.class.getName());
for (int i = 0; i < listCount; i++) {
// 1 means the item is present.
dest.writeInt(1);
list.get(i).writeToParcel(dest, 0);
}
}
public abstract static class BaseObject implements Parcelable {
protected static final int TYPE_SMALL = 0;
protected static final int TYPE_LARGE = 1;
protected void writeToParcel(Parcel dest, int flags, int type) {
dest.writeInt(type);
}
@Override
public int describeContents() {
return 0;
}
/**
* This is *REALLY* bad, but we're doing it in the test to ensure that we handle
* the possible exploit when unparceling an object with the BaseObject written as
* Creator.
*/
public static final Creator<BaseObject> CREATOR = new Creator<BaseObject>() {
@Override
public BaseObject createFromParcel(Parcel source) {
switch (source.readInt()) {
case TYPE_SMALL:
return SmallObject.createFromParcelBody(source);
case TYPE_LARGE:
return LargeObject.createFromParcelBody(source);
default:
throw new IllegalArgumentException("Unknown type");
}
}
@Override
public BaseObject[] newArray(int size) {
return new BaseObject[size];
}
};
}
public static class SmallObject extends BaseObject {
public int mFieldA;
public int mFieldB;
public SmallObject(int a, int b) {
mFieldA = a;
mFieldB = b;
}
@Override
public void writeToParcel(Parcel dest, int flags) {
super.writeToParcel(dest, flags, TYPE_SMALL);
dest.writeInt(mFieldA);
dest.writeInt(mFieldB);
}
public static SmallObject createFromParcelBody(Parcel source) {
return new SmallObject(source.readInt(), source.readInt());
}
public static final Creator<SmallObject> CREATOR = new Creator<SmallObject>() {
@Override
public SmallObject createFromParcel(Parcel source) {
// Consume the type (as it is always written out).
source.readInt();
return createFromParcelBody(source);
}
@Override
public SmallObject[] newArray(int size) {
return new SmallObject[size];
}
};
}
public static class LargeObject extends BaseObject {
public int mFieldA;
public int mFieldB;
public int mFieldC;
public int mFieldD;
public int mFieldE;
public LargeObject(int a, int b, int c, int d, int e) {
mFieldA = a;
mFieldB = b;
mFieldC = c;
mFieldD = d;
mFieldE = e;
}
@Override
public void writeToParcel(Parcel dest, int flags) {
super.writeToParcel(dest, flags, TYPE_LARGE);
dest.writeInt(mFieldA);
dest.writeInt(mFieldB);
dest.writeInt(mFieldC);
dest.writeInt(mFieldD);
dest.writeInt(mFieldE);
}
public static LargeObject createFromParcelBody(Parcel source) {
return new LargeObject(
source.readInt(),
source.readInt(),
source.readInt(),
source.readInt(),
source.readInt()
);
}
public static final Creator<LargeObject> CREATOR = new Creator<LargeObject>() {
@Override
public LargeObject createFromParcel(Parcel source) {
// Consume the type (as it is always written out).
source.readInt();
return createFromParcelBody(source);
}
@Override
public LargeObject[] newArray(int size) {
return new LargeObject[size];
}
};
}
}