drgreen/android_frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

am 571c5a26: Merge "Fix UB in ResourceTable::stringToInt."

Browse Source 
* commit '571c5a26f767f67118f34ca0d426aeeb4a287b2c':
  Fix UB in ResourceTable::stringToInt.
This commit is contained in:
Dan Albert
2015-04-14 23:44:51 +00:00
committed by Android Git Automerger
parent 6a38c86dd8 571c5a26f7
commit 44d7989abf
4 changed files with 155 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
include/androidfw/ResourceTypes.h
View File

@ -372,7 +372,8 @@ struct Res_value
};
// The data for this item, as interpreted according to dataType.
uint32_t data;
typedef uint32_t data_type;
data_type data;
void copyFrom_dtoh(const Res_value& src);
};
@ -1502,6 +1503,8 @@ private:
KeyedVector<String16, uint8_t> mEntries;
};
bool U16StringToInt(const char16_t* s, size_t len, Res_value* outValue);
/**
* Convenience class for accessing data in a ResTable resource.
*/

72
libs/androidfw/ResourceTypes.cpp
View File

@ -17,6 +17,16 @@
#define LOG_TAG "ResourceType"
//#define LOG_NDEBUG 0
#include <ctype.h>
#include <memory.h>
#include <stddef.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <limits>
#include <type_traits>
#include <androidfw/ByteBucketArray.h>
#include <androidfw/ResourceTypes.h>
#include <androidfw/TypeWrappers.h>
@ -27,17 +37,10 @@
#include <utils/String16.h>
#include <utils/String8.h>
#ifdef HAVE_ANDROID_OS
#ifdef __ANDROID__
#include <binder/TextOutput.h>
#endif
#include <stdlib.h>
#include <string.h>
#include <memory.h>
#include <ctype.h>
#include <stdint.h>
#include <stddef.h>
#ifndef INT32_MAX
#define INT32_MAX ((int32_t)(2147483647))
#endif
@ -4551,8 +4554,7 @@ static bool parse_unit(const char* str, Res_value* outValue,
return false;
}
bool ResTable::stringToInt(const char16_t* s, size_t len, Res_value* outValue)
bool U16StringToInt(const char16_t* s, size_t len, Res_value* outValue)
{
while (len > 0 && isspace16(*s)) {
s++;
@ -4564,7 +4566,7 @@ bool ResTable::stringToInt(const char16_t* s, size_t len, Res_value* outValue)
}
size_t i = 0;
int32_t val = 0;
int64_t val = 0;
bool neg = false;
if (*s == '-') {
@ -4576,28 +4578,50 @@ bool ResTable::stringToInt(const char16_t* s, size_t len, Res_value* outValue)
return false;
}
static_assert(std::is_same<uint32_t, Res_value::data_type>::value,
"Res_value::data_type has changed. The range checks in this "
"function are no longer correct.");
// Decimal or hex?
if (s[i] == '0' && s[i+1] == 'x') {
if (outValue)
outValue->dataType = outValue->TYPE_INT_HEX;
bool isHex;
if (len > 1 && s[i] == '0' && s[i+1] == 'x') {
isHex = true;
i += 2;
if (neg) {
return false;
}
if (i == len) {
// Just u"0x"
return false;
}
bool error = false;
while (i < len && !error) {
val = (val*16) + get_hex(s[i], &error);
i++;
if (val > std::numeric_limits<uint32_t>::max()) {
return false;
}
}
if (error) {
return false;
}
} else {
if (outValue)
outValue->dataType = outValue->TYPE_INT_DEC;
isHex = false;
while (i < len) {
if (s[i] < '0' || s[i] > '9') {
return false;
}
val = (val*10) + s[i]-'0';
i++;
if ((neg && -val < std::numeric_limits<int32_t>::min()) ||
(!neg && val > std::numeric_limits<int32_t>::max())) {
return false;
}
}
}
@ -4607,13 +4631,21 @@ bool ResTable::stringToInt(const char16_t* s, size_t len, Res_value* outValue)
i++;
}
if (i == len) {
if (outValue)
outValue->data = val;
if (i != len) {
return false;
}
if (outValue) {
outValue->dataType =
isHex ? outValue->TYPE_INT_HEX : outValue->TYPE_INT_DEC;
outValue->data = static_cast<Res_value::data_type>(val);
}
return true;
}
return false;
bool ResTable::stringToInt(const char16_t* s, size_t len, Res_value* outValue)
{
return U16StringToInt(s, len, outValue);
}
bool ResTable::stringToFloat(const char16_t* s, size_t len, Res_value* outValue)

30
libs/androidfw/tests/Android.mk
View File

@ -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 \

81
libs/androidfw/tests/ResTable_test.cpp
View File

@ -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);
}
}