StringBlock incremental hardening default values

When a string cannot be retrieved from a StringBlock due to pages of
the StringPool missing, rather than throwing an
IndexOutOfBoundsException, compute a default value rather that
abides by the API contacts of the functions that use the StringBlock.

IndexOutOfBoundsExceptions are not really documented as a possible
exception that can be thrown from APIs that use StringBlock because
it indicates that the binary resource table was built incorrectly.

Returning default values is much less likely to break applications
that fetch the resources of incrementally installed applications than
throwing IndexOutOfBoundsExceptions where they would normally not be
expected.

Bug: 188174746
Test: atest ResourcesHardeningTest
Change-Id: I58fe754fb446fefc031ddba19e290830fdd6d015

6 files changed, 74 insertions, 20 deletions

diff --git a/core/java/android/content/res/ApkAssets.java b/core/java/android/content/res/ApkAssets.java
index f3783e4a1328..224ff14c5df4 100644
--- a/core/java/android/content/res/ApkAssets.java
+++ b/core/java/android/content/res/ApkAssets.java
@@ -334,13 +334,14 @@ public final class ApkAssets {
         }
     }
 
+    @Nullable
     CharSequence getStringFromPool(int idx) {
         if (mStringBlock == null) {
             return null;
         }
 
         synchronized (this) {
-            return mStringBlock.get(idx);
+            return mStringBlock.getSequence(idx);
         }
     }
 
diff --git a/core/java/android/content/res/AssetManager.java b/core/java/android/content/res/AssetManager.java
index 13433dc7979f..d50404e94544 100644
--- a/core/java/android/content/res/AssetManager.java
+++ b/core/java/android/content/res/AssetManager.java
@@ -550,7 +550,9 @@ public final class AssetManager implements AutoCloseable {
                     outValue.changingConfigurations);
 
             if (outValue.type == TypedValue.TYPE_STRING) {
-                outValue.string = getPooledStringForCookie(cookie, outValue.data);
+                if ((outValue.string = getPooledStringForCookie(cookie, outValue.data)) == null) {
+                    return false;
+                }
             }
             return true;
         }
@@ -731,7 +733,9 @@ public final class AssetManager implements AutoCloseable {
                     outValue.changingConfigurations);
 
             if (outValue.type == TypedValue.TYPE_STRING) {
-                outValue.string = getPooledStringForCookie(cookie, outValue.data);
+                if ((outValue.string = getPooledStringForCookie(cookie, outValue.data)) == null) {
+                    return false;
+                }
             }
             return true;
         }
@@ -833,6 +837,7 @@ public final class AssetManager implements AutoCloseable {
         }
     }
 
+    @Nullable
     CharSequence getPooledStringForCookie(int cookie, int id) {
         // Cookies map to ApkAssets starting at 1.
         return getApkAssets()[cookie - 1].getStringFromPool(id);
diff --git a/core/java/android/content/res/StringBlock.java b/core/java/android/content/res/StringBlock.java
index 8f3f77b99739..5bc235f0eeba 100644
--- a/core/java/android/content/res/StringBlock.java
+++ b/core/java/android/content/res/StringBlock.java
@@ -16,6 +16,8 @@
 
 package android.content.res;
 
+import android.annotation.NonNull;
+import android.annotation.Nullable;
 import android.compat.annotation.UnsupportedAppUsage;
 import android.graphics.Color;
 import android.graphics.Paint;
@@ -86,8 +88,19 @@ public final class StringBlock implements Closeable {
                 + ": " + nativeGetSize(mNative));
     }
 
+    /**
+     * @deprecated use {@link #getSequence(int)} which can return null when a string cannot be found
+     *             due to incremental installation.
+     */
+    @Deprecated
     @UnsupportedAppUsage
     public CharSequence get(int idx) {
+        CharSequence seq = getSequence(idx);
+        return seq == null ? "" : seq;
+    }
+
+    @Nullable
+    public CharSequence getSequence(int idx) {
         synchronized (this) {
             if (mStrings != null) {
                 CharSequence res = mStrings[idx];
@@ -108,6 +121,9 @@ public final class StringBlock implements Closeable {
                 }
             }
             String str = nativeGetString(mNative, idx);
+            if (str == null) {
+                return null;
+            }
             CharSequence res = str;
             int[] style = nativeGetStyle(mNative, idx);
             if (localLOGV) Log.v(TAG, "Got string: " + str);
@@ -133,6 +149,9 @@ public final class StringBlock implements Closeable {
                     }
 
                     String styleTag = nativeGetString(mNative, styleId);
+                    if (styleTag == null) {
+                        return null;
+                    }
 
                     if (styleTag.equals("b")) {
                         mStyleIDs.boldId = styleId;
@@ -161,8 +180,10 @@ public final class StringBlock implements Closeable {
 
                 res = applyStyles(str, style, mStyleIDs);
             }
-            if (mStrings != null) mStrings[idx] = res;
-            else mSparseStrings.put(idx, res);
+            if (res != null) {
+                if (mStrings != null) mStrings[idx] = res;
+                else mSparseStrings.put(idx, res);
+            }
             return res;
         }
     }
@@ -203,6 +224,7 @@ public final class StringBlock implements Closeable {
         private int marqueeId = -1;
     }
 
+    @Nullable
     private CharSequence applyStyles(String str, int[] style, StyleIDs ids) {
         if (style.length == 0)
             return str;
@@ -260,6 +282,9 @@ public final class StringBlock implements Closeable {
                                Spannable.SPAN_INCLUSIVE_INCLUSIVE);
             } else {
                 String tag = nativeGetString(mNative, type);
+                if (tag == null) {
+                    return null;
+                }
 
                 if (tag.startsWith("font;")) {
                     String sub;
diff --git a/core/java/android/content/res/TypedArray.java b/core/java/android/content/res/TypedArray.java
index 2a349e9a4600..d6f55d65ead5 100644
--- a/core/java/android/content/res/TypedArray.java
+++ b/core/java/android/content/res/TypedArray.java
@@ -1376,6 +1376,7 @@ public class TypedArray implements AutoCloseable {
         return true;
     }
 
+    @Nullable
     private CharSequence loadStringValueAt(int index) {
         final int[] data = mData;
         final int cookie = data[index + STYLE_ASSET_COOKIE];
diff --git a/core/java/android/content/res/XmlBlock.java b/core/java/android/content/res/XmlBlock.java
index b0291ce6d842..53dfed374fbc 100644
--- a/core/java/android/content/res/XmlBlock.java
+++ b/core/java/android/content/res/XmlBlock.java
@@ -19,6 +19,7 @@ package android.content.res;
 import static android.content.res.Resources.ID_NULL;
 
 import android.annotation.AnyRes;
+import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.compat.annotation.UnsupportedAppUsage;
 import android.os.Build;
@@ -161,9 +162,10 @@ public final class XmlBlock implements AutoCloseable {
         public int getDepth() {
             return mDepth;
         }
+        @Nullable
         public String getText() {
             int id = nativeGetText(mParseState);
-            return id >= 0 ? mStrings.get(id).toString() : null;
+            return id >= 0 ? getSequenceString(mStrings.getSequence(id)) : null;
         }
         public int getLineNumber() {
             return nativeGetLineNumber(mParseState);
@@ -189,25 +191,29 @@ public final class XmlBlock implements AutoCloseable {
             }
             return chars;
         }
+        @Nullable
         public String getNamespace() {
             int id = nativeGetNamespace(mParseState);
-            return id >= 0 ? mStrings.get(id).toString() : "";
+            return id >= 0 ? getSequenceString(mStrings.getSequence(id)) : "";
         }
+        @Nullable
         public String getName() {
             int id = nativeGetName(mParseState);
-            return id >= 0 ? mStrings.get(id).toString() : null;
+            return id >= 0 ? getSequenceString(mStrings.getSequence(id)) : null;
         }
+        @NonNull
         public String getAttributeNamespace(int index) {
             int id = nativeGetAttributeNamespace(mParseState, index);
             if (DEBUG) System.out.println("getAttributeNamespace of " + index + " = " + id);
-            if (id >= 0) return mStrings.get(id).toString();
+            if (id >= 0) return getSequenceString(mStrings.getSequence(id));
             else if (id == -1) return "";
             throw new IndexOutOfBoundsException(String.valueOf(index));
         }
+        @NonNull
         public String getAttributeName(int index) {
             int id = nativeGetAttributeName(mParseState, index);
             if (DEBUG) System.out.println("getAttributeName of " + index + " = " + id);
-            if (id >= 0) return mStrings.get(id).toString();
+            if (id >= 0) return getSequenceString(mStrings.getSequence(id));
             throw new IndexOutOfBoundsException(String.valueOf(index));
         }
         public String getAttributePrefix(int index) {
@@ -220,10 +226,11 @@ public final class XmlBlock implements AutoCloseable {
         public int getAttributeCount() {
             return mEventType == START_TAG ? nativeGetAttributeCount(mParseState) : -1;
         }
+        @NonNull
         public String getAttributeValue(int index) {
             int id = nativeGetAttributeStringValue(mParseState, index);
             if (DEBUG) System.out.println("getAttributeValue of " + index + " = " + id);
-            if (id >= 0) return mStrings.get(id).toString();
+            if (id >= 0) return getSequenceString(mStrings.getSequence(id));
 
             // May be some other type...  check and try to convert if so.
             int t = nativeGetAttributeDataType(mParseState, index);
@@ -390,7 +397,7 @@ public final class XmlBlock implements AutoCloseable {
             int v = nativeGetAttributeData(mParseState, idx);
             if (t == TypedValue.TYPE_STRING) {
                 return XmlUtils.convertValueToList(
-                    mStrings.get(v), options, defaultValue);
+                    mStrings.getSequence(v), options, defaultValue);
             }
             return v;
         }
@@ -444,14 +451,15 @@ public final class XmlBlock implements AutoCloseable {
             }
             throw new RuntimeException("not a float!");
         }
-
+        @Nullable
         public String getIdAttribute() {
             int id = nativeGetIdAttribute(mParseState);
-            return id >= 0 ? mStrings.get(id).toString() : null;
+            return id >= 0 ? getSequenceString(mStrings.getSequence(id)) : null;
         }
+        @Nullable
         public String getClassAttribute() {
             int id = nativeGetClassAttribute(mParseState);
-            return id >= 0 ? mStrings.get(id).toString() : null;
+            return id >= 0 ? getSequenceString(mStrings.getSequence(id)) : null;
         }
 
         public int getIdAttributeResourceValue(int defaultValue) {
@@ -463,6 +471,17 @@ public final class XmlBlock implements AutoCloseable {
             return nativeGetStyleAttribute(mParseState);
         }
 
+        private String getSequenceString(@Nullable CharSequence str) {
+            if (str == null) {
+                // A value of null retrieved from a StringPool indicates that retrieval of the
+                // string failed due to incremental installation. The presence of all the XmlBlock
+                // data is verified when it is created, so this exception must not be possible.
+                throw new IllegalStateException("Retrieving a string from the StringPool of an"
+                        + " XmlBlock should never fail");
+            }
+            return str.toString();
+        }
+
         public void close() {
             synchronized (mBlock) {
                 if (mParseState != 0) {
@@ -472,13 +491,14 @@ public final class XmlBlock implements AutoCloseable {
                 }
             }
         }
-        
+
         protected void finalize() throws Throwable {
             close();
         }
 
+        @Nullable
         /*package*/ final CharSequence getPooledString(int id) {
-            return mStrings.get(id);
+            return mStrings.getSequence(id);
         }
 
         @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
diff --git a/core/jni/android_util_StringBlock.cpp b/core/jni/android_util_StringBlock.cpp
index d94e0841003f..dd7df26ee881 100644
--- a/core/jni/android_util_StringBlock.cpp
+++ b/core/jni/android_util_StringBlock.cpp
@@ -72,7 +72,7 @@ static jstring android_content_StringBlock_nativeGetString(JNIEnv* env, jobject
     ResStringPool* osb = reinterpret_cast<ResStringPool*>(token);
     if (osb == NULL) {
         jniThrowNullPointerException(env, NULL);
-        return 0;
+        return NULL;
     }
 
     if (auto str8 = osb->string8At(idx); str8.has_value()) {
@@ -80,9 +80,11 @@ static jstring android_content_StringBlock_nativeGetString(JNIEnv* env, jobject
     }
 
     auto str = osb->stringAt(idx);
-    if (UNLIKELY(!str.has_value())) {
+    if (IsIOError(str)) {
+        return NULL;
+    } else if (UNLIKELY(!str.has_value())) {
         jniThrowException(env, "java/lang/IndexOutOfBoundsException", NULL);
-        return 0;
+        return NULL;
     }
 
     return env->NewString((const jchar*)str->data(), str->size());