CVE-2020-10531: Integer Overflow or Wraparound (CWE-190)
value: An issue was discovered in International Components for Unicode (ICU) for C/C++ through 66.1. An integer overflow, leading to a heap-based buffer overflow, exists in the UnicodeString::doAppend() function in common/unistr.cpp.
CWE description: The product performs a calculation that can
         produce an integer overflow or wraparound when the logic
         assumes that the resulting value will always be larger than
         the original value. This occurs when an integer value is
         incremented to a value that is too large to store in the
         associated representation. When this occurs, the value may
         become a very small or negative number.
CVSS v3 base score: 8.8
Published: 2020-03-12T19:15Z
Repository: https://github.com/unicode-org/icu
Language: C++
Files changed: icu4c/source/common/unistr.cpp, icu4c/source/test/intltest/ustrtest.cpp, icu4c/source/test/intltest/ustrtest.h
Commit date: 2020-02-03 13:22:30 -0800
Commit message: ICU-20958 Prevent SEGV_MAPERR in append

See #971
Diff: diff --git a/icu4c/source/common/unistr.cpp b/icu4c/source/common/unistr.cpp
index 901bb3358b..077b4d6ef2 100644
--- a/icu4c/source/common/unistr.cpp
+++ b/icu4c/source/common/unistr.cpp
@@ -1561,11 +1561,15 @@ UnicodeString::doAppend(const UChar *srcChars, int32_t srcStart, int32_t srcLeng
       return *this;
     }
   }
 
   int32_t oldLength = length();
-  int32_t newLength = oldLength + srcLength;
+  int32_t newLength;
+  if (uprv_add32_overflow(oldLength, srcLength, &newLength)) {
+    setToBogus();
+    return *this;
+  }
 
   // Check for append onto ourself
   const UChar* oldArray = getArrayStart();
   if (isBufferWritable() &&
       oldArray < srcChars + srcLength &&
diff --git a/icu4c/source/test/intltest/ustrtest.cpp b/icu4c/source/test/intltest/ustrtest.cpp
index b6515ea813..ad38bdf53a 100644
--- a/icu4c/source/test/intltest/ustrtest.cpp
+++ b/icu4c/source/test/intltest/ustrtest.cpp
@@ -65,10 +65,11 @@ void UnicodeStringTest::runIndexedTest( int32_t index, UBool exec, const char* &
     TESTCASE_AUTO(TestMoveSwap);
     TESTCASE_AUTO(TestUInt16Pointers);
     TESTCASE_AUTO(TestWCharPointers);
     TESTCASE_AUTO(TestNullPointers);
     TESTCASE_AUTO(TestUnicodeStringInsertAppendToSelf);
+    TESTCASE_AUTO(TestLargeAppend);
     TESTCASE_AUTO_END;
 }
 
 void
 UnicodeStringTest::TestBasicManipulation()
@@ -2308,5 +2309,66 @@ void UnicodeStringTest::TestUnicodeStringInsertAppendToSelf() {
     str = UnicodeString(TRUE, u"abcde", 5);
     sub = str.tempSubString(1, 3);
     str.insert(2, sub);
     assertEquals("", u"abbcdcde", str);
 }
+
+void UnicodeStringTest::TestLargeAppend() {
+    if(quick) return;
+
+    IcuTestErrorCode status(*this, "TestLargeAppend");
+    // Make a large UnicodeString
+    int32_t len = 0xAFFFFFF;
+    UnicodeString str;
+    char16_t *buf = str.getBuffer(len);
+    // A fast way to set buffer to valid Unicode.
+    // 4E4E is a valid unicode character
+    uprv_memset(buf, 0x4e, len * 2);
+    str.releaseBuffer(len);
+    UnicodeString dest;
+    // Append it 16 times
+    // 0xAFFFFFF times 16 is 0xA4FFFFF1,
+    // which is greater than INT32_MAX, which is 0x7FFFFFFF.
+    int64_t total = 0;
+    for (int32_t i = 0; i < 16; i++) {
+        dest.append(str);
+        total += len;
+        if (total <= INT32_MAX) {
+            assertFalse("dest is not bogus", dest.isBogus());
+        } else {
+            assertTrue("dest should be bogus", dest.isBogus());
+        }
+    }
+    dest.remove();
+    total = 0;
+    for (int32_t i = 0; i < 16; i++) {
+        dest.append(str);
+        total += len;
+        if (total + len <= INT32_MAX) {
+            assertFalse("dest is not bogus", dest.isBogus());
+        } else if (total <= INT32_MAX) {
+            // Check that a string of exactly the maximum size works
+            UnicodeString str2;
+            int32_t remain = INT32_MAX - total;
+            char16_t *buf2 = str2.getBuffer(remain);
+            if (buf2 == nullptr) {
+                // if somehow memory allocation fail, return the test
+                return;
+            }
+            uprv_memset(buf2, 0x4e, remain * 2);
+            str2.releaseBuffer(remain);
+            dest.append(str2);
+            total += remain;
+            assertEquals("When a string of exactly the maximum size works", (int64_t)INT32_MAX, total);
+            assertEquals("When a string of exactly the maximum size works", INT32_MAX, dest.length());
+            assertFalse("dest is not bogus", dest.isBogus());
+
+            // Check that a string size+1 goes bogus
+            str2.truncate(1);
+            dest.append(str2);
+            total++;
+            assertTrue("dest should be bogus", dest.isBogus());
+        } else {
+            assertTrue("dest should be bogus", dest.isBogus());
+        }
+    }
+}
diff --git a/icu4c/source/test/intltest/ustrtest.h b/icu4c/source/test/intltest/ustrtest.h
index 218befdcc6..4a356a92c7 100644
--- a/icu4c/source/test/intltest/ustrtest.h
+++ b/icu4c/source/test/intltest/ustrtest.h
@@ -95,8 +95,9 @@ class UnicodeStringTest: public IntlTest {
 
     void TestUInt16Pointers();
     void TestWCharPointers();
     void TestNullPointers();
     void TestUnicodeStringInsertAppendToSelf();
+    void TestLargeAppend();
 };
 
 #endif

Vulnerable code:   int32_t newLength = oldLength + srcLength;
Fixed code:   int32_t newLength;
  if (uprv_add32_overflow(oldLength, srcLength, &newLength)) {
    setToBogus();
    return *this;
  }
    TESTCASE_AUTO(TestLargeAppend);

void UnicodeStringTest::TestLargeAppend() {
    if(quick) return;

    IcuTestErrorCode status(*this, "TestLargeAppend");
    // Make a large UnicodeString
    int32_t len = 0xAFFFFFF;
    UnicodeString str;
    char16_t *buf = str.getBuffer(len);
    // A fast way to set buffer to valid Unicode.
    // 4E4E is a valid unicode character
    uprv_memset(buf, 0x4e, len * 2);
    str.releaseBuffer(len);
    UnicodeString dest;
    // Append it 16 times
    // 0xAFFFFFF times 16 is 0xA4FFFFF1,
    // which is greater than INT32_MAX, which is 0x7FFFFFFF.
    int64_t total = 0;
    for (int32_t i = 0; i < 16; i++) {
        dest.append(str);
        total += len;
        if (total <= INT32_MAX) {
            assertFalse("dest is not bogus", dest.isBogus());
        } else {
            assertTrue("dest should be bogus", dest.isBogus());
        }
    }
    dest.remove();
    total = 0;
    for (int32_t i = 0; i < 16; i++) {
        dest.append(str);
        total += len;
        if (total + len <= INT32_MAX) {
            assertFalse("dest is not bogus", dest.isBogus());
        } else if (total <= INT32_MAX) {
            // Check that a string of exactly the maximum size works
            UnicodeString str2;
            int32_t remain = INT32_MAX - total;
            char16_t *buf2 = str2.getBuffer(remain);
            if (buf2 == nullptr) {
                // if somehow memory allocation fail, return the test
                return;
            }
            uprv_memset(buf2, 0x4e, remain * 2);
            str2.releaseBuffer(remain);
            dest.append(str2);
            total += remain;
            assertEquals("When a string of exactly the maximum size works", (int64_t)INT32_MAX, total);
            assertEquals("When a string of exactly the maximum size works", INT32_MAX, dest.length());
            assertFalse("dest is not bogus", dest.isBogus());

            // Check that a string size+1 goes bogus
            str2.truncate(1);
            dest.append(str2);
            total++;
            assertTrue("dest should be bogus", dest.isBogus());
        } else {
            assertTrue("dest should be bogus", dest.isBogus());
        }
    }
}
    void TestLargeAppend();