Merge "RESTRICT AUTOMERGE: Drop invalid data." into tm-dev

1 files changed, 34 insertions, 8 deletions

diff --git a/core/java/android/os/PersistableBundle.java b/core/java/android/os/PersistableBundle.java
index f4edcb1317f4..acfd15cb005e 100644
--- a/core/java/android/os/PersistableBundle.java
+++ b/core/java/android/os/PersistableBundle.java
@@ -21,6 +21,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.util.ArrayMap;
+import android.util.Slog;
 import android.util.TypedXmlPullParser;
 import android.util.TypedXmlSerializer;
 import android.util.Xml;
@@ -50,6 +51,8 @@ import java.util.ArrayList;
  */
 public final class PersistableBundle extends BaseBundle implements Cloneable, Parcelable,
         XmlUtils.WriteMapCallback {
+    private static final String TAG = "PersistableBundle";
+
     private static final String TAG_PERSISTABLEMAP = "pbundle_as_map";
 
     /** An unmodifiable {@code PersistableBundle} that is always {@link #isEmpty() empty}. */
@@ -118,7 +121,11 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa
      * @hide
      */
     public PersistableBundle(Bundle b) {
-        this(b.getItemwiseMap());
+        this(b, true);
+    }
+
+    private PersistableBundle(Bundle b, boolean throwException) {
+        this(b.getItemwiseMap(), throwException);
     }
 
     /**
@@ -127,7 +134,7 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa
      * @param map a Map containing only those items that can be persisted.
      * @throws IllegalArgumentException if any element of #map cannot be persisted.
      */
-    private PersistableBundle(ArrayMap<String, Object> map) {
+    private PersistableBundle(ArrayMap<String, Object> map, boolean throwException) {
         super();
         mFlags = FLAG_DEFUSABLE;
 
@@ -136,16 +143,23 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa
 
         // Now verify each item throwing an exception if there is a violation.
         final int N = mMap.size();
-        for (int i=0; i<N; i++) {
+        for (int i = N - 1; i >= 0; --i) {
             Object value = mMap.valueAt(i);
             if (value instanceof ArrayMap) {
                 // Fix up any Maps by replacing them with PersistableBundles.
-                mMap.setValueAt(i, new PersistableBundle((ArrayMap<String, Object>) value));
+                mMap.setValueAt(i,
+                        new PersistableBundle((ArrayMap<String, Object>) value, throwException));
             } else if (value instanceof Bundle) {
-                mMap.setValueAt(i, new PersistableBundle(((Bundle) value)));
+                mMap.setValueAt(i, new PersistableBundle((Bundle) value, throwException));
             } else if (!isValidType(value)) {
-                throw new IllegalArgumentException("Bad value in PersistableBundle key="
-                        + mMap.keyAt(i) + " value=" + value);
+                final String errorMsg = "Bad value in PersistableBundle key="
+                        + mMap.keyAt(i) + " value=" + value;
+                if (throwException) {
+                    throw new IllegalArgumentException(errorMsg);
+                } else {
+                    Slog.wtfStack(TAG, errorMsg);
+                    mMap.removeAt(i);
+                }
             }
         }
     }
@@ -268,6 +282,15 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa
     /** @hide */
     public void saveToXml(TypedXmlSerializer out) throws IOException, XmlPullParserException {
         unparcel();
+        // Explicitly drop invalid types an attacker may have added before persisting.
+        for (int i = mMap.size() - 1; i >= 0; --i) {
+            final Object value = mMap.valueAt(i);
+            if (!isValidType(value)) {
+                Slog.e(TAG, "Dropping bad data before persisting: "
+                        + mMap.keyAt(i) + "=" + value);
+                mMap.removeAt(i);
+            }
+        }
         XmlUtils.writeMapXml(mMap, out, this);
     }
 
@@ -322,9 +345,12 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa
         while (((event = in.next()) != XmlPullParser.END_DOCUMENT) &&
                 (event != XmlPullParser.END_TAG || in.getDepth() < outerDepth)) {
             if (event == XmlPullParser.START_TAG) {
+                // Don't throw an exception when restoring from XML since an attacker could try to
+                // input invalid data in the persisted file.
                 return new PersistableBundle((ArrayMap<String, Object>)
                         XmlUtils.readThisArrayMapXml(in, startTag, tagName,
-                        new MyReadMapCallback()));
+                        new MyReadMapCallback()),
+                        /* throwException */ false);
             }
         }
         return new PersistableBundle();  // An empty mutable PersistableBundle