From 59682761fb4ea24b91292bc53cfe9fbf5e3cfc63 Mon Sep 17 00:00:00 2001
From: Elliott Hughes <enh@google.com>
Date: Thu, 10 Jun 2021 16:42:20 -0700
Subject: [PATCH] Check for overflow in String8::real_append.

Bug: http://b/178822418
Test: new tests
Change-Id: I73631a070ade0689441abe5645ba5a5f64a58675
---
 libutils/String8.cpp      | 26 ++++++++++++++------------
 libutils/String8_test.cpp | 14 +++++++++++++-
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/libutils/String8.cpp b/libutils/String8.cpp
index df160445e..b391b1a18 100644
--- a/libutils/String8.cpp
+++ b/libutils/String8.cpp
@@ -327,21 +327,23 @@ status_t String8::appendFormatV(const char* fmt, va_list args)
     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)
diff --git a/libutils/String8_test.cpp b/libutils/String8_test.cpp
index 9efcc6fa4..1356cd08f 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 @@ TEST_F(String8Test, ValidUtf16Conversion) {
     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);
 }