From a6be6f0acb955fc1917496820521ec41cf1e5557 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Thu, 10 Jun 2021 17:06:26 -0700 Subject: [PATCH] Check for overflow in String16::append and String16::insert. Bug: http://b/178802681 Bug: http://b/178821065 Test: new tests Change-Id: I2352ea4c65e3f29e44e2ad6cad20ad610ceace1f --- libutils/String16.cpp | 116 ++++++++++++------------------------- libutils/String16_test.cpp | 36 +++++++++++- 2 files changed, 72 insertions(+), 80 deletions(-) diff --git a/libutils/String16.cpp b/libutils/String16.cpp index faf90c233..d08b212fd 100644 --- a/libutils/String16.cpp +++ b/libutils/String16.cpp @@ -186,99 +186,59 @@ status_t String16::setTo(const char16_t* other, size_t len) return NO_MEMORY; } -status_t String16::append(const String16& other) -{ - const size_t myLen = size(); - const size_t otherLen = other.size(); - if (myLen == 0) { - setTo(other); - return OK; - } else if (otherLen == 0) { - return OK; - } - - if (myLen >= SIZE_MAX / sizeof(char16_t) - otherLen) { - android_errorWriteLog(0x534e4554, "73826242"); - abort(); - } - - SharedBuffer* buf = - static_cast(editResize((myLen + otherLen + 1) * sizeof(char16_t))); - if (buf) { - char16_t* str = (char16_t*)buf->data(); - memcpy(str+myLen, other, (otherLen+1)*sizeof(char16_t)); - mString = str; - return OK; - } - return NO_MEMORY; +status_t String16::append(const String16& other) { + return append(other.string(), other.size()); } -status_t String16::append(const char16_t* chrs, size_t otherLen) -{ +status_t String16::append(const char16_t* chrs, size_t otherLen) { const size_t myLen = size(); - if (myLen == 0) { - setTo(chrs, otherLen); - return OK; - } else if (otherLen == 0) { - return OK; - } - if (myLen >= SIZE_MAX / sizeof(char16_t) - otherLen) { - android_errorWriteLog(0x534e4554, "73826242"); - abort(); - } + if (myLen == 0) return setTo(chrs, otherLen); - SharedBuffer* buf = - static_cast(editResize((myLen + otherLen + 1) * sizeof(char16_t))); - if (buf) { - char16_t* str = (char16_t*)buf->data(); - memcpy(str+myLen, chrs, otherLen*sizeof(char16_t)); - str[myLen+otherLen] = 0; - mString = str; - return OK; - } - return NO_MEMORY; + if (otherLen == 0) return OK; + + size_t size = myLen; + if (__builtin_add_overflow(size, otherLen, &size) || + __builtin_add_overflow(size, 1, &size) || + __builtin_mul_overflow(size, sizeof(char16_t), &size)) return NO_MEMORY; + + SharedBuffer* buf = static_cast(editResize(size)); + if (!buf) return NO_MEMORY; + + char16_t* str = static_cast(buf->data()); + memcpy(str + myLen, chrs, otherLen * sizeof(char16_t)); + str[myLen + otherLen] = 0; + mString = str; + return OK; } -status_t String16::insert(size_t pos, const char16_t* chrs) -{ +status_t String16::insert(size_t pos, const char16_t* chrs) { return insert(pos, chrs, strlen16(chrs)); } -status_t String16::insert(size_t pos, const char16_t* chrs, size_t len) -{ +status_t String16::insert(size_t pos, const char16_t* chrs, size_t otherLen) { const size_t myLen = size(); - if (myLen == 0) { - return setTo(chrs, len); - return OK; - } else if (len == 0) { - return OK; - } + + if (myLen == 0) return setTo(chrs, otherLen); + + if (otherLen == 0) return OK; if (pos > myLen) pos = myLen; - #if 0 - printf("Insert in to %s: pos=%d, len=%d, myLen=%d, chrs=%s\n", - String8(*this).string(), pos, - len, myLen, String8(chrs, len).string()); - #endif + size_t size = myLen; + if (__builtin_add_overflow(size, otherLen, &size) || + __builtin_add_overflow(size, 1, &size) || + __builtin_mul_overflow(size, sizeof(char16_t), &size)) return NO_MEMORY; - SharedBuffer* buf = - static_cast(editResize((myLen + len + 1) * sizeof(char16_t))); - if (buf) { - char16_t* str = (char16_t*)buf->data(); - if (pos < myLen) { - memmove(str+pos+len, str+pos, (myLen-pos)*sizeof(char16_t)); - } - memcpy(str+pos, chrs, len*sizeof(char16_t)); - str[myLen+len] = 0; - mString = str; - #if 0 - printf("Result (%d chrs): %s\n", size(), String8(*this).string()); - #endif - return OK; - } - return NO_MEMORY; + SharedBuffer* buf = static_cast(editResize(size)); + if (!buf) return NO_MEMORY; + + char16_t* str = static_cast(buf->data()); + if (pos < myLen) memmove(str + pos + otherLen, str + pos, (myLen - pos) * sizeof(char16_t)); + memcpy(str + pos, chrs, otherLen * sizeof(char16_t)); + str[myLen + otherLen] = 0; + mString = str; + return OK; } ssize_t String16::findFirst(char16_t c) const diff --git a/libutils/String16_test.cpp b/libutils/String16_test.cpp index 54662ac41..c4783211f 100644 --- a/libutils/String16_test.cpp +++ b/libutils/String16_test.cpp @@ -19,7 +19,7 @@ #include -namespace android { +using namespace android; ::testing::AssertionResult Char16_tStringEquals(const char16_t* a, const char16_t* b) { if (strcmp16(a, b) != 0) { @@ -197,4 +197,36 @@ TEST(String16Test, ValidUtf8Conversion) { EXPECT_STR16EQ(another, u"abcdef"); } -} // namespace android +TEST(String16Test, append) { + String16 s; + EXPECT_EQ(OK, s.append(String16(u"foo"))); + EXPECT_STR16EQ(u"foo", s); + EXPECT_EQ(OK, s.append(String16(u"bar"))); + EXPECT_STR16EQ(u"foobar", s); + EXPECT_EQ(OK, s.append(u"baz", 0)); + EXPECT_STR16EQ(u"foobar", s); + EXPECT_EQ(NO_MEMORY, s.append(u"baz", SIZE_MAX)); + EXPECT_STR16EQ(u"foobar", s); +} + +TEST(String16Test, insert) { + String16 s; + + // Inserting into the empty string inserts at the start. + EXPECT_EQ(OK, s.insert(123, u"foo")); + EXPECT_STR16EQ(u"foo", s); + + // Inserting zero characters at any position is okay, but won't expand the string. + EXPECT_EQ(OK, s.insert(123, u"foo", 0)); + EXPECT_STR16EQ(u"foo", s); + + // Inserting past the end of a non-empty string appends. + EXPECT_EQ(OK, s.insert(123, u"bar")); + EXPECT_STR16EQ(u"foobar", s); + + EXPECT_EQ(OK, s.insert(3, u"!")); + EXPECT_STR16EQ(u"foo!bar", s); + + EXPECT_EQ(NO_MEMORY, s.insert(3, u"", SIZE_MAX)); + EXPECT_STR16EQ(u"foo!bar", s); +}