From 99b25d2817a1058e56c5384a43040e0f3f291ce1 Mon Sep 17 00:00:00 2001
From: Sunny Goyal <sunnygoyal@google.com>
Date: Wed, 1 Nov 2017 11:58:13 -0700
Subject: [PATCH] Preventing recursive referrence in drawables

Bug: 68706673
Bug: 66498711
Test: Added CTS tests
Change-Id: I8034f49d16f9a7bc1749714fd6d6231bba5088d0
---
 .../android/content/res/ResourcesImpl.java    | 66 +++++++++++++++----
 1 file changed, 55 insertions(+), 11 deletions(-)

diff --git a/core/java/android/content/res/ResourcesImpl.java b/core/java/android/content/res/ResourcesImpl.java
index 386239cf4f93a..3239212adf66c 100644
--- a/core/java/android/content/res/ResourcesImpl.java
+++ b/core/java/android/content/res/ResourcesImpl.java
@@ -49,6 +49,8 @@ import android.util.TypedValue;
 import android.util.Xml;
 import android.view.DisplayAdjustments;
 
+import com.android.internal.util.GrowingArrayUtils;
+
 import org.xmlpull.v1.XmlPullParser;
 import org.xmlpull.v1.XmlPullParserException;
 
@@ -117,6 +119,13 @@ public class ResourcesImpl {
     private final ConfigurationBoundResourceCache<StateListAnimator> mStateListAnimatorCache =
             new ConfigurationBoundResourceCache<>();
 
+    // A stack of all the resourceIds already referenced when parsing a resource. This is used to
+    // detect circular references in the xml.
+    // Using a ThreadLocal variable ensures that we have different stacks for multiple parallel
+    // calls to ResourcesImpl
+    private final ThreadLocal<LookupStack> mLookupStack =
+            ThreadLocal.withInitial(() -> new LookupStack());
+
     /** Size of the cyclical cache used to map XML files to blocks. */
     private static final int XML_BLOCK_CACHE_SIZE = 4;
 
@@ -784,19 +793,29 @@ public class ResourcesImpl {
         final Drawable dr;
 
         Trace.traceBegin(Trace.TRACE_TAG_RESOURCES, file);
+        LookupStack stack = mLookupStack.get();
         try {
-            if (file.endsWith(".xml")) {
-                final XmlResourceParser rp = loadXmlResourceParser(
-                        file, id, value.assetCookie, "drawable");
-                dr = Drawable.createFromXmlForDensity(wrapper, rp, density, theme);
-                rp.close();
-            } else {
-                final InputStream is = mAssets.openNonAsset(
-                        value.assetCookie, file, AssetManager.ACCESS_STREAMING);
-                dr = Drawable.createFromResourceStream(wrapper, value, is, file, null);
-                is.close();
+            // Perform a linear search to check if we have already referenced this resource before.
+            if (stack.contains(id)) {
+                throw new Exception("Recursive reference in drawable");
             }
-        } catch (Exception | StackOverflowError e) {
+            stack.push(id);
+            try {
+                if (file.endsWith(".xml")) {
+                    final XmlResourceParser rp = loadXmlResourceParser(
+                            file, id, value.assetCookie, "drawable");
+                    dr = Drawable.createFromXmlForDensity(wrapper, rp, density, theme);
+                    rp.close();
+                } else {
+                    final InputStream is = mAssets.openNonAsset(
+                            value.assetCookie, file, AssetManager.ACCESS_STREAMING);
+                    dr = Drawable.createFromResourceStream(wrapper, value, is, file, null);
+                    is.close();
+                }
+            } finally {
+                stack.pop();
+            }
+        } catch (Exception e) {
             Trace.traceEnd(Trace.TRACE_TAG_RESOURCES);
             final NotFoundException rnf = new NotFoundException(
                     "File " + file + " from drawable resource ID #0x" + Integer.toHexString(id));
@@ -1377,4 +1396,29 @@ public class ResourcesImpl {
             }
         }
     }
+
+    private static class LookupStack {
+
+        // Pick a reasonable default size for the array, it is grown as needed.
+        private int[] mIds = new int[4];
+        private int mSize = 0;
+
+        public void push(int id) {
+            mIds = GrowingArrayUtils.append(mIds, mSize, id);
+            mSize++;
+        }
+
+        public boolean contains(int id) {
+            for (int i = 0; i < mSize; i++) {
+                if (mIds[i] == id) {
+                    return true;
+                }
+            }
+            return false;
+        }
+
+        public void pop() {
+            mSize--;
+        }
+    }
 }