commit 59682761fb4ea24b91292bc53cfe9fbf5e3cfc63
author Elliott Hughes <enh@google.com> Thu Jun 10 16:42:20 2021 -0700
committer Elliott Hughes <enh@google.com> Thu Jun 10 16:42:20 2021 -0700
tree b0908677474a373189fdca003eb4f9eee1b5cafc
parent 8f654d8a99738d096e2a7bf87324a515ec0c33bc

Check for overflow in String8::real_append.

Bug: http://b/178822418
Test: new tests
Change-Id: I73631a070ade0689441abe5645ba5a5f64a58675

libutils/String8.cpp

diff --git a/libutils/String8.cpp b/libutils/String8.cpp
index df16044..b391b1a 100644
--- a/libutils/String8.cpp
+++ b/libutils/String8.cpp
@@ -327,21 +327,23 @@
     return result;
 }
 
-status_t String8::real_append(const char* other, size_t otherLen)
-{
+status_t String8::real_append(const char* other, size_t otherLen) {
     const size_t myLen = bytes();
 
-    SharedBuffer* buf = SharedBuffer::bufferFromData(mString)
-        ->editResize(myLen+otherLen+1);
-    if (buf) {
-        char* str = (char*)buf->data();
-        mString = str;
-        str += myLen;
-        memcpy(str, other, otherLen);
-        str[otherLen] = '\0';
-        return OK;
+    SharedBuffer* buf;
+    size_t newLen;
+    if (__builtin_add_overflow(myLen, otherLen, &newLen) ||
+        __builtin_add_overflow(newLen, 1, &newLen) ||
+        (buf = SharedBuffer::bufferFromData(mString)->editResize(newLen)) == nullptr) {
+        return NO_MEMORY;
     }
-    return NO_MEMORY;
+
+    char* str = (char*)buf->data();
+    mString = str;
+    str += myLen;
+    memcpy(str, other, otherLen);
+    str[otherLen] = '\0';
+    return OK;
 }
 
 char* String8::lockBuffer(size_t size)

libutils/String8_test.cpp

diff --git a/libutils/String8_test.cpp b/libutils/String8_test.cpp
index 9efcc6f..1356cd0 100644
--- a/libutils/String8_test.cpp
+++ b/libutils/String8_test.cpp
@@ -15,13 +15,14 @@
  */
 
 #define LOG_TAG "String8_test"
+
 #include <utils/Log.h>
 #include <utils/String8.h>
 #include <utils/String16.h>
 
 #include <gtest/gtest.h>
 
-namespace android {
+using namespace android;
 
 class String8Test : public testing::Test {
 protected:
@@ -101,4 +102,15 @@
     String8 valid = String8(String16(tmp));
     EXPECT_STREQ(valid, "abcdef");
 }
+
+TEST_F(String8Test, append) {
+    String8 s;
+    EXPECT_EQ(OK, s.append("foo"));
+    EXPECT_STREQ("foo", s);
+    EXPECT_EQ(OK, s.append("bar"));
+    EXPECT_STREQ("foobar", s);
+    EXPECT_EQ(OK, s.append("baz", 0));
+    EXPECT_STREQ("foobar", s);
+    EXPECT_EQ(NO_MEMORY, s.append("baz", SIZE_MAX));
+    EXPECT_STREQ("foobar", s);
 }