Merge "Added decoding of truncated AAPT string lengths." into pi-dev

5 files changed, 123 insertions, 36 deletions

diff --git a/libs/androidfw/ResourceTypes.cpp b/libs/androidfw/ResourceTypes.cpp
index 6268c40f34b4..985a7569a92c 100644
--- a/libs/androidfw/ResourceTypes.cpp
+++ b/libs/androidfw/ResourceTypes.cpp
@@ -727,44 +727,28 @@ const char16_t* ResStringPool::stringAt(size_t idx, size_t* u16len) const
                 if ((uint32_t)(u8str+u8len-strings) < mStringPoolSize) {
                     AutoMutex lock(mDecodeLock);
 
-                    if (mCache == NULL) {
-#ifndef __ANDROID__
-                        if (kDebugStringPoolNoisy) {
-                            ALOGI("CREATING STRING CACHE OF %zu bytes",
-                                    mHeader->stringCount*sizeof(char16_t**));
-                        }
-#else
-                        // We do not want to be in this case when actually running Android.
-                        ALOGW("CREATING STRING CACHE OF %zu bytes",
-                                static_cast<size_t>(mHeader->stringCount*sizeof(char16_t**)));
-#endif
-                        mCache = (char16_t**)calloc(mHeader->stringCount, sizeof(char16_t*));
-                        if (mCache == NULL) {
-                            ALOGW("No memory trying to allocate decode cache table of %d bytes\n",
-                                    (int)(mHeader->stringCount*sizeof(char16_t**)));
-                            return NULL;
-                        }
+                    if (mCache != NULL && mCache[idx] != NULL) {
+                        return mCache[idx];
                     }
 
-                    if (mCache[idx] != NULL) {
-                        return mCache[idx];
+                    // Retrieve the actual length of the utf8 string if the
+                    // encoded length was truncated
+                    if (stringDecodeAt(idx, u8str, u8len, &u8len) == NULL) {
+                        return NULL;
                     }
 
+                    // Since AAPT truncated lengths longer than 0x7FFF, check
+                    // that the bits that remain after truncation at least match
+                    // the bits of the actual length
                     ssize_t actualLen = utf8_to_utf16_length(u8str, u8len);
-                    if (actualLen < 0 || (size_t)actualLen != *u16len) {
+                    if (actualLen < 0 || ((size_t)actualLen & 0x7FFF) != *u16len) {
                         ALOGW("Bad string block: string #%lld decoded length is not correct "
                                 "%lld vs %llu\n",
                                 (long long)idx, (long long)actualLen, (long long)*u16len);
                         return NULL;
                     }
 
-                    // Reject malformed (non null-terminated) strings
-                    if (u8str[u8len] != 0x00) {
-                        ALOGW("Bad string block: string #%d is not null-terminated",
-                              (int)idx);
-                        return NULL;
-                    }
-
+                    *u16len = (size_t) actualLen;
                     char16_t *u16str = (char16_t *)calloc(*u16len+1, sizeof(char16_t));
                     if (!u16str) {
                         ALOGW("No memory when trying to allocate decode cache for string #%d\n",
@@ -772,10 +756,31 @@ const char16_t* ResStringPool::stringAt(size_t idx, size_t* u16len) const
                         return NULL;
                     }
 
+                    utf8_to_utf16(u8str, u8len, u16str, *u16len + 1);
+
+                    if (mCache == NULL) {
+#ifndef __ANDROID__
+                        if (kDebugStringPoolNoisy) {
+                            ALOGI("CREATING STRING CACHE OF %zu bytes",
+                                  mHeader->stringCount*sizeof(char16_t**));
+                        }
+#else
+                        // We do not want to be in this case when actually running Android.
+                        ALOGW("CREATING STRING CACHE OF %zu bytes",
+                                static_cast<size_t>(mHeader->stringCount*sizeof(char16_t**)));
+#endif
+                        mCache = (char16_t**)calloc(mHeader->stringCount, sizeof(char16_t*));
+                        if (mCache == NULL) {
+                            ALOGW("No memory trying to allocate decode cache table of %d bytes\n",
+                                  (int)(mHeader->stringCount*sizeof(char16_t**)));
+                            return NULL;
+                        }
+                    }
+
                     if (kDebugStringPoolNoisy) {
-                        ALOGI("Caching UTF8 string: %s", u8str);
+                      ALOGI("Caching UTF8 string: %s", u8str);
                     }
-                    utf8_to_utf16(u8str, u8len, u16str, *u16len + 1);
+
                     mCache[idx] = u16str;
                     return u16str;
                 } else {
@@ -812,13 +817,8 @@ const char* ResStringPool::string8At(size_t idx, size_t* outLen) const
             *outLen = encLen;
 
             if ((uint32_t)(str+encLen-strings) < mStringPoolSize) {
-                // Reject malformed (non null-terminated) strings
-                if (str[encLen] != 0x00) {
-                    ALOGW("Bad string block: string #%d is not null-terminated",
-                          (int)idx);
-                    return NULL;
-                }
-              return (const char*)str;
+                return stringDecodeAt(idx, str, encLen, outLen);
+
             } else {
                 ALOGW("Bad string block: string #%d extends to %d, past end at %d\n",
                         (int)idx, (int)(str+encLen-strings), (int)mStringPoolSize);
@@ -832,6 +832,38 @@ const char* ResStringPool::string8At(size_t idx, size_t* outLen) const
     return NULL;
 }
 
+/**
+ * AAPT incorrectly writes a truncated string length when the string size
+ * exceeded the maximum possible encode length value (0x7FFF). To decode a
+ * truncated length, iterate through length values that end in the encode length
+ * bits. Strings that exceed the maximum encode length are not placed into
+ * StringPools in AAPT2.
+ **/
+const char* ResStringPool::stringDecodeAt(size_t idx, const uint8_t* str,
+                                          const size_t encLen, size_t* outLen) const {
+    const uint8_t* strings = (uint8_t*)mStrings;
+
+    size_t i = 0, end = encLen;
+    while ((uint32_t)(str+end-strings) < mStringPoolSize) {
+        if (str[end] == 0x00) {
+            if (i != 0) {
+                ALOGW("Bad string block: string #%d is truncated (actual length is %d)",
+                      (int)idx, (int)end);
+            }
+
+            *outLen = end;
+            return (const char*)str;
+        }
+
+        end = (++i << (sizeof(uint8_t) * 8 * 2 - 1)) | encLen;
+    }
+
+    // Reject malformed (non null-terminated) strings
+    ALOGW("Bad string block: string #%d is not null-terminated",
+          (int)idx);
+    return NULL;
+}
+
 const String8 ResStringPool::string8ObjectAt(size_t idx) const
 {
     size_t len;
diff --git a/libs/androidfw/include/androidfw/ResourceTypes.h b/libs/androidfw/include/androidfw/ResourceTypes.h
index a1f15f0c96cb..bc0a07a512f9 100644
--- a/libs/androidfw/include/androidfw/ResourceTypes.h
+++ b/libs/androidfw/include/androidfw/ResourceTypes.h
@@ -538,6 +538,9 @@ private:
     uint32_t                    mStringPoolSize;    // number of uint16_t
     const uint32_t*             mStyles;
     uint32_t                    mStylePoolSize;    // number of uint32_t
+
+    const char* stringDecodeAt(size_t idx, const uint8_t* str, const size_t encLen,
+                               size_t* outLen) const;
 };
 
 /**
diff --git a/libs/androidfw/tests/ResTable_test.cpp b/libs/androidfw/tests/ResTable_test.cpp
index 2df41305237e..326474e16e5d 100644
--- a/libs/androidfw/tests/ResTable_test.cpp
+++ b/libs/androidfw/tests/ResTable_test.cpp
@@ -424,4 +424,56 @@ TEST(ResTableTest, GetConfigurationsReturnsUniqueList) {
   EXPECT_EQ(1, std::count(locales.begin(), locales.end(), String8("sv")));
 }
 
+TEST(ResTableTest, TruncatedEncodeLength) {
+  std::string contents;
+  ASSERT_TRUE(ReadFileFromZipToString(GetTestDataPath() + "/length_decode/length_decode_valid.apk",
+                                      "resources.arsc", &contents));
+
+  ResTable table;
+  ASSERT_EQ(NO_ERROR, table.add(contents.data(), contents.size()));
+
+  Res_value val;
+  ssize_t block = table.getResource(0x7f010001, &val, MAY_NOT_BE_BAG);
+  ASSERT_GE(block, 0);
+  ASSERT_EQ(Res_value::TYPE_STRING, val.dataType);
+
+  const ResStringPool* pool = table.getTableStringBlock(block);
+  ASSERT_TRUE(pool != NULL);
+  ASSERT_LT(val.data, pool->size());
+
+  // Make sure a string with a truncated length is read to its correct length
+  size_t str_len;
+  const char* target_str8 = pool->string8At(val.data, &str_len);
+  ASSERT_TRUE(target_str8 != NULL);
+  ASSERT_EQ(size_t(40076), String8(target_str8, str_len).size());
+  ASSERT_EQ(target_str8[40075], ']');
+
+  const char16_t* target_str16 = pool->stringAt(val.data, &str_len);
+  ASSERT_TRUE(target_str16 != NULL);
+  ASSERT_EQ(size_t(40076), String16(target_str16, str_len).size());
+  ASSERT_EQ(target_str8[40075], (char16_t) ']');
+
+  // Load an edited apk with the null terminator removed from the end of the
+  // string
+  std::string invalid_contents;
+  ASSERT_TRUE(ReadFileFromZipToString(GetTestDataPath() + "/length_decode/length_decode_invalid.apk",
+                                      "resources.arsc", &invalid_contents));
+  ResTable invalid_table;
+  ASSERT_EQ(NO_ERROR, invalid_table.add(invalid_contents.data(), invalid_contents.size()));
+
+  Res_value invalid_val;
+  ssize_t invalid_block = invalid_table.getResource(0x7f010001, &invalid_val, MAY_NOT_BE_BAG);
+  ASSERT_GE(invalid_block, 0);
+  ASSERT_EQ(Res_value::TYPE_STRING, invalid_val.dataType);
+
+  const ResStringPool* invalid_pool = invalid_table.getTableStringBlock(invalid_block);
+  ASSERT_TRUE(invalid_pool != NULL);
+  ASSERT_LT(invalid_val.data, invalid_pool->size());
+
+  // Make sure a string with a truncated length that is not null terminated errors
+  // and does not return the string
+  ASSERT_TRUE(invalid_pool->string8At(invalid_val.data, &str_len) == NULL);
+  ASSERT_TRUE(invalid_pool->stringAt(invalid_val.data, &str_len) == NULL);
+}
+
 }  // namespace android
diff --git a/libs/androidfw/tests/data/length_decode/length_decode_invalid.apk b/libs/androidfw/tests/data/length_decode/length_decode_invalid.apk
new file mode 100644
index 000000000000..b089651e6786
--- /dev/null
+++ b/libs/androidfw/tests/data/length_decode/length_decode_invalid.apk
diff --git a/libs/androidfw/tests/data/length_decode/length_decode_valid.apk b/libs/androidfw/tests/data/length_decode/length_decode_valid.apk
new file mode 100644
index 000000000000..add23e1ee6d4
--- /dev/null
+++ b/libs/androidfw/tests/data/length_decode/length_decode_valid.apk