Check for overflow in String16::append and String16::insert.

Bug: http://b/178802681
Bug: http://b/178821065
Test: new tests
Change-Id: I2352ea4c65e3f29e44e2ad6cad20ad610ceace1f

2 files changed, 71 insertions, 79 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)
-{
+status_t String16::append(const String16& other) {
+    return append(other.string(), other.size());
+}
+
+status_t String16::append(const char16_t* chrs, size_t otherLen) {
     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();
-    }
+    if (myLen == 0) return setTo(chrs, otherLen);
 
-    SharedBuffer* buf =
-            static_cast<SharedBuffer*>(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;
-}
+    if (otherLen == 0) return OK;
 
-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;
-    }
+    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;
 
-    if (myLen >= SIZE_MAX / sizeof(char16_t) - otherLen) {
-        android_errorWriteLog(0x534e4554, "73826242");
-        abort();
-    }
+    SharedBuffer* buf = static_cast<SharedBuffer*>(editResize(size));
+    if (!buf) return NO_MEMORY;
 
-    SharedBuffer* buf =
-            static_cast<SharedBuffer*>(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;
+    char16_t* str = static_cast<char16_t*>(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<SharedBuffer*>(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<SharedBuffer*>(editResize(size));
+    if (!buf) return NO_MEMORY;
+
+    char16_t* str = static_cast<char16_t*>(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 <gtest/gtest.h>
 
-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);
+}