Andrew Geissler | 4b740dc | 2020-05-05 08:54:39 -0500 | [diff] [blame] | 1 | From b7d08bc04a4296982fcef8b6b8a354a9e4e7afca Mon Sep 17 00:00:00 2001 |
| 2 | From: Frank Tang <ftang@chromium.org> |
| 3 | Date: Sat, 1 Feb 2020 02:39:04 +0000 |
| 4 | Subject: [PATCH] ICU-20958 Prevent SEGV_MAPERR in append |
| 5 | |
| 6 | See #971 |
| 7 | |
| 8 | Upstream-Status: Accepted |
| 9 | CVE: CVE-2020-10531 |
| 10 | |
| 11 | Reference to upstream patch: |
| 12 | https://github.com/unicode-org/icu/commit/b7d08bc04a4296982fcef8b6b8a354a9e4e7afca |
| 13 | |
| 14 | --- |
| 15 | common/unistr.cpp | 6 ++- |
| 16 | test/intltest/ustrtest.cpp | 62 +++++++++++++++++++++++++++++++ |
| 17 | test/intltest/ustrtest.h | 1 + |
| 18 | 3 files changed, 68 insertions(+), 1 deletion(-) |
| 19 | |
| 20 | diff --git a/common/unistr.cpp b/common/unistr.cpp |
| 21 | index 901bb33..6ea0915 100644 |
| 22 | --- a/common/unistr.cpp |
| 23 | +++ b/common/unistr.cpp |
| 24 | @@ -1563,7 +1563,11 @@ UnicodeString::doAppend(const UChar *srcChars, int32_t srcStart, int32_t srcLeng |
| 25 | } |
| 26 | |
| 27 | int32_t oldLength = length(); |
| 28 | - int32_t newLength = oldLength + srcLength; |
| 29 | + int32_t newLength; |
| 30 | + if (uprv_add32_overflow(oldLength, srcLength, &newLength)) { |
| 31 | + setToBogus(); |
| 32 | + return *this; |
| 33 | + } |
| 34 | |
| 35 | // Check for append onto ourself |
| 36 | const UChar* oldArray = getArrayStart(); |
| 37 | diff --git a/test/intltest/ustrtest.cpp b/test/intltest/ustrtest.cpp |
| 38 | index b6515ea..ad38bdf 100644 |
| 39 | --- a/test/intltest/ustrtest.cpp |
| 40 | +++ b/test/intltest/ustrtest.cpp |
| 41 | @@ -67,6 +67,7 @@ void UnicodeStringTest::runIndexedTest( int32_t index, UBool exec, const char* & |
| 42 | TESTCASE_AUTO(TestWCharPointers); |
| 43 | TESTCASE_AUTO(TestNullPointers); |
| 44 | TESTCASE_AUTO(TestUnicodeStringInsertAppendToSelf); |
| 45 | + TESTCASE_AUTO(TestLargeAppend); |
| 46 | TESTCASE_AUTO_END; |
| 47 | } |
| 48 | |
| 49 | @@ -2310,3 +2311,64 @@ void UnicodeStringTest::TestUnicodeStringInsertAppendToSelf() { |
| 50 | str.insert(2, sub); |
| 51 | assertEquals("", u"abbcdcde", str); |
| 52 | } |
| 53 | + |
| 54 | +void UnicodeStringTest::TestLargeAppend() { |
| 55 | + if(quick) return; |
| 56 | + |
| 57 | + IcuTestErrorCode status(*this, "TestLargeAppend"); |
| 58 | + // Make a large UnicodeString |
| 59 | + int32_t len = 0xAFFFFFF; |
| 60 | + UnicodeString str; |
| 61 | + char16_t *buf = str.getBuffer(len); |
| 62 | + // A fast way to set buffer to valid Unicode. |
| 63 | + // 4E4E is a valid unicode character |
| 64 | + uprv_memset(buf, 0x4e, len * 2); |
| 65 | + str.releaseBuffer(len); |
| 66 | + UnicodeString dest; |
| 67 | + // Append it 16 times |
| 68 | + // 0xAFFFFFF times 16 is 0xA4FFFFF1, |
| 69 | + // which is greater than INT32_MAX, which is 0x7FFFFFFF. |
| 70 | + int64_t total = 0; |
| 71 | + for (int32_t i = 0; i < 16; i++) { |
| 72 | + dest.append(str); |
| 73 | + total += len; |
| 74 | + if (total <= INT32_MAX) { |
| 75 | + assertFalse("dest is not bogus", dest.isBogus()); |
| 76 | + } else { |
| 77 | + assertTrue("dest should be bogus", dest.isBogus()); |
| 78 | + } |
| 79 | + } |
| 80 | + dest.remove(); |
| 81 | + total = 0; |
| 82 | + for (int32_t i = 0; i < 16; i++) { |
| 83 | + dest.append(str); |
| 84 | + total += len; |
| 85 | + if (total + len <= INT32_MAX) { |
| 86 | + assertFalse("dest is not bogus", dest.isBogus()); |
| 87 | + } else if (total <= INT32_MAX) { |
| 88 | + // Check that a string of exactly the maximum size works |
| 89 | + UnicodeString str2; |
| 90 | + int32_t remain = INT32_MAX - total; |
| 91 | + char16_t *buf2 = str2.getBuffer(remain); |
| 92 | + if (buf2 == nullptr) { |
| 93 | + // if somehow memory allocation fail, return the test |
| 94 | + return; |
| 95 | + } |
| 96 | + uprv_memset(buf2, 0x4e, remain * 2); |
| 97 | + str2.releaseBuffer(remain); |
| 98 | + dest.append(str2); |
| 99 | + total += remain; |
| 100 | + assertEquals("When a string of exactly the maximum size works", (int64_t)INT32_MAX, total); |
| 101 | + assertEquals("When a string of exactly the maximum size works", INT32_MAX, dest.length()); |
| 102 | + assertFalse("dest is not bogus", dest.isBogus()); |
| 103 | + |
| 104 | + // Check that a string size+1 goes bogus |
| 105 | + str2.truncate(1); |
| 106 | + dest.append(str2); |
| 107 | + total++; |
| 108 | + assertTrue("dest should be bogus", dest.isBogus()); |
| 109 | + } else { |
| 110 | + assertTrue("dest should be bogus", dest.isBogus()); |
| 111 | + } |
| 112 | + } |
| 113 | +} |
| 114 | diff --git a/test/intltest/ustrtest.h b/test/intltest/ustrtest.h |
| 115 | index 218befd..4a356a9 100644 |
| 116 | --- a/test/intltest/ustrtest.h |
| 117 | +++ b/test/intltest/ustrtest.h |
| 118 | @@ -97,6 +97,7 @@ public: |
| 119 | void TestWCharPointers(); |
| 120 | void TestNullPointers(); |
| 121 | void TestUnicodeStringInsertAppendToSelf(); |
| 122 | + void TestLargeAppend(); |
| 123 | }; |
| 124 | |
| 125 | #endif |
| 126 | -- |
| 127 | 2.17.1 |
| 128 | |