Fix UB in ResourceTable::stringToInt.

Was here because UBsan found integer overflow in the parsing for hex
numbers, since hex numbers here are actually unsigned but assigned to
a signed integer.

Also fixes a number of missing error conditions.

Change-Id: Iaea576daedfc6c75521cde02de3fe9dd0198a3b7

2 files changed, 98 insertions, 13 deletions

diff --git a/libs/androidfw/tests/Android.mk b/libs/androidfw/tests/Android.mk
index 58b78b5..a353575 100644
--- a/libs/androidfw/tests/Android.mk
+++ b/libs/androidfw/tests/Android.mk
@@ -19,6 +19,7 @@
 # targets here.
 # ==========================================================
 LOCAL_PATH:= $(call my-dir)
+
 testFiles := \
     AttributeFinder_test.cpp \
     ByteBucketArray_test.cpp \
@@ -32,27 +33,33 @@ testFiles := \
     TypeWrappers_test.cpp \
     ZipUtils_test.cpp
 
+androidfw_test_cflags := \
+    -Wall \
+    -Werror \
+    -Wunused \
+    -Wunreachable-code \
+    -Wno-missing-field-initializers \
+
+# gtest is broken.
+androidfw_test_cflags += -Wno-unnamed-type-template-args
+
 # ==========================================================
 # Build the host tests: libandroidfw_tests
 # ==========================================================
 include $(CLEAR_VARS)
 
 LOCAL_MODULE := libandroidfw_tests
-
-LOCAL_CFLAGS += -Wall -Werror -Wunused -Wunreachable-code
-# gtest is broken.
-LOCAL_CFLAGS += -Wno-unnamed-type-template-args
-
+LOCAL_CFLAGS := $(androidfw_test_cflags)
 LOCAL_SRC_FILES := $(testFiles)
 LOCAL_STATIC_LIBRARIES := \
     libandroidfw \
     libutils \
     libcutils \
-    liblog
+    liblog \
+    libz \
 
 include $(BUILD_HOST_NATIVE_TEST)
 
-
 # ==========================================================
 # Build the device tests: libandroidfw_tests
 # ==========================================================
@@ -60,14 +67,11 @@ ifneq ($(SDK_ONLY),true)
 include $(CLEAR_VARS)
 
 LOCAL_MODULE := libandroidfw_tests
-
-LOCAL_CFLAGS += -Wall -Werror -Wunused -Wunreachable-code
-# gtest is broken.
-LOCAL_CFLAGS += -Wno-unnamed-type-template-args
-
+LOCAL_CFLAGS := $(androidfw_test_cflags)
 LOCAL_SRC_FILES := $(testFiles) \
     BackupData_test.cpp \
-    ObbFile_test.cpp
+    ObbFile_test.cpp \
+
 LOCAL_SHARED_LIBRARIES := \
     libandroidfw \
     libcutils \
diff --git a/libs/androidfw/tests/ResTable_test.cpp b/libs/androidfw/tests/ResTable_test.cpp
index 6a9314e..dcfe91e 100644
--- a/libs/androidfw/tests/ResTable_test.cpp
+++ b/libs/androidfw/tests/ResTable_test.cpp
@@ -16,6 +16,10 @@
 
 #include <androidfw/ResourceTypes.h>
 
+#include <codecvt>
+#include <locale>
+#include <string>
+
 #include <utils/String8.h>
 #include <utils/String16.h>
 #include "TestHelpers.h"
@@ -201,4 +205,81 @@ TEST(ResTableTest, emptyTableHasSensibleDefaults) {
     ASSERT_LT(table.getResource(base::R::integer::number1, &val, MAY_NOT_BE_BAG), 0);
 }
 
+void testU16StringToInt(const char16_t* str, uint32_t expectedValue,
+                        bool expectSuccess, bool expectHex) {
+    size_t len = std::char_traits<char16_t>::length(str);
+
+    // Gtest can't print UTF-16 strings, so we have to convert to UTF-8 :(
+    std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t> convert;
+    std::string s = convert.to_bytes(std::u16string(str, len));
+
+    Res_value out = {};
+    ASSERT_EQ(expectSuccess, U16StringToInt(str, len, &out))
+        << "Failed with " << s;
+
+    if (!expectSuccess) {
+        ASSERT_EQ(out.TYPE_NULL, out.dataType) << "Failed with " << s;
+        return;
+    }
+
+    if (expectHex) {
+        ASSERT_EQ(out.TYPE_INT_HEX, out.dataType) << "Failed with " << s;
+    } else {
+        ASSERT_EQ(out.TYPE_INT_DEC, out.dataType) << "Failed with " << s;
+    }
+
+    ASSERT_EQ(expectedValue, out.data) << "Failed with " << s;
+}
+
+TEST(ResTableTest, U16StringToInt) {
+    testU16StringToInt(u"", 0U, false, false);
+    testU16StringToInt(u"    ", 0U, false, false);
+    testU16StringToInt(u"\t\n", 0U, false, false);
+
+    testU16StringToInt(u"abcd", 0U, false, false);
+    testU16StringToInt(u"10abcd", 0U, false, false);
+    testU16StringToInt(u"42 42", 0U, false, false);
+    testU16StringToInt(u"- 42", 0U, false, false);
+    testU16StringToInt(u"-", 0U, false, false);
+
+    testU16StringToInt(u"0x", 0U, false, true);
+    testU16StringToInt(u"0xnope", 0U, false, true);
+    testU16StringToInt(u"0X42", 0U, false, true);
+    testU16StringToInt(u"0x42 0x42", 0U, false, true);
+    testU16StringToInt(u"-0x0", 0U, false, true);
+    testU16StringToInt(u"-0x42", 0U, false, true);
+    testU16StringToInt(u"- 0x42", 0U, false, true);
+
+    // Note that u" 42" would pass. This preserves the old behavior, but it may
+    // not be desired.
+    testU16StringToInt(u"42 ", 0U, false, false);
+    testU16StringToInt(u"0x42 ", 0U, false, true);
+
+    // Decimal cases.
+    testU16StringToInt(u"0", 0U, true, false);
+    testU16StringToInt(u"-0", 0U, true, false);
+    testU16StringToInt(u"42", 42U, true, false);
+    testU16StringToInt(u" 42", 42U, true, false);
+    testU16StringToInt(u"-42", static_cast<uint32_t>(-42), true, false);
+    testU16StringToInt(u" -42", static_cast<uint32_t>(-42), true, false);
+    testU16StringToInt(u"042", 42U, true, false);
+    testU16StringToInt(u"-042", static_cast<uint32_t>(-42), true, false);
+
+    // Hex cases.
+    testU16StringToInt(u"0x0", 0x0, true, true);
+    testU16StringToInt(u"0x42", 0x42, true, true);
+    testU16StringToInt(u" 0x42", 0x42, true, true);
+
+    // Just before overflow cases:
+    testU16StringToInt(u"2147483647", INT_MAX, true, false);
+    testU16StringToInt(u"-2147483648", static_cast<uint32_t>(INT_MIN), true,
+                       false);
+    testU16StringToInt(u"0xffffffff", UINT_MAX, true, true);
+
+    // Overflow cases:
+    testU16StringToInt(u"2147483648", 0U, false, false);
+    testU16StringToInt(u"-2147483649", 0U, false, false);
+    testU16StringToInt(u"0x1ffffffff", 0U, false, true);
+}
+
 }