1From e450fa50fc242282551f56b941dc93b9a8a0bcbb Mon Sep 17 00:00:00 2001 2From: Frank Tang <ftang@chromium.org> 3Date: Tue, 13 Apr 2021 15:16:50 -0700 4Subject: [PATCH] ICU-21587 Fix memory bug w/ baseName 5 6Edge cases not fixed in assign and move assign operator 7while the locale is long and call setKeywordValue with incorrect 8keyword/values. 9--- 10 icu4c/source/common/locid.cpp | 11 +++++++++-- 11 icu4c/source/test/intltest/loctest.cpp | 26 ++++++++++++++++++++++++++ 12 icu4c/source/test/intltest/loctest.h | 2 ++ 13 3 files changed, 37 insertions(+), 2 deletions(-) 14 15diff --git a/icu4c/source/common/locid.cpp b/icu4c/source/common/locid.cpp 16index 02cd82a7b8e..3c6e5b06690 100644 17--- a/icu4c/source/common/locid.cpp 18+++ b/icu4c/source/common/locid.cpp 19@@ -469,14 +469,18 @@ Locale& Locale::operator=(Locale&& other) U_NOEXCEPT { 20 if ((baseName != fullName) && (baseName != fullNameBuffer)) uprv_free(baseName); 21 if (fullName != fullNameBuffer) uprv_free(fullName); 22 23- if (other.fullName == other.fullNameBuffer) { 24+ if (other.fullName == other.fullNameBuffer || other.baseName == other.fullNameBuffer) { 25 uprv_strcpy(fullNameBuffer, other.fullNameBuffer); 26+ } 27+ if (other.fullName == other.fullNameBuffer) { 28 fullName = fullNameBuffer; 29 } else { 30 fullName = other.fullName; 31 } 32 33- if (other.baseName == other.fullName) { 34+ if (other.baseName == other.fullNameBuffer) { 35+ baseName = fullNameBuffer; 36+ } else if (other.baseName == other.fullName) { 37 baseName = fullName; 38 } else { 39 baseName = other.baseName; 40@@ -2681,6 +2685,9 @@ Locale::setKeywordValue(const char* keywordName, const char* keywordValue, UErro 41 if (fullName != fullNameBuffer) { 42 // if full Name is already on the heap, need to free it. 43 uprv_free(fullName); 44+ if (baseName == fullName) { 45+ baseName = newFullName; // baseName should not point to freed memory. 46+ } 47 } 48 fullName = newFullName; 49 status = U_ZERO_ERROR; 50diff --git a/icu4c/source/test/intltest/loctest.cpp b/icu4c/source/test/intltest/loctest.cpp 51index ce41a4c00e7..5503b008b0c 100644 52--- a/icu4c/source/test/intltest/loctest.cpp 53+++ b/icu4c/source/test/intltest/loctest.cpp 54@@ -284,6 +284,8 @@ void LocaleTest::runIndexedTest( int32_t index, UBool exec, const char* &name, c 55 TESTCASE_AUTO(TestSetUnicodeKeywordValueNullInLongLocale); 56 TESTCASE_AUTO(TestCanonicalize); 57 TESTCASE_AUTO(TestLeak21419); 58+ TESTCASE_AUTO(TestLongLocaleSetKeywordAssign); 59+ TESTCASE_AUTO(TestLongLocaleSetKeywordMoveAssign); 60 TESTCASE_AUTO_END; 61 } 62 63@@ -6520,6 +6522,30 @@ void LocaleTest::TestSetUnicodeKeywordValueInLongLocale() { 64 } 65 } 66 67+void LocaleTest::TestLongLocaleSetKeywordAssign() { 68+ IcuTestErrorCode status(*this, "TestLongLocaleSetKeywordAssign"); 69+ // A long base name, with an illegal keyword and copy constructor 70+ icu::Locale l("de_AAAAAAA1_AAAAAAA2_AAAAAAA3_AAAAAAA4_AAAAAAA5_AAAAAAA6_" 71+ "AAAAAAA7_AAAAAAA8_AAAAAAA9_AAAAAA10_AAAAAA11_AAAAAA12_" 72+ "AAAAAA13_AAAAAA14_AAAAAA15_AAAAAA16_AAAAAA17_AAAAAA18"); 73+ Locale l2; 74+ l.setUnicodeKeywordValue("co", "12", status); // Cause an error 75+ status.reset(); 76+ l2 = l; // copy operator on such bogus locale. 77+} 78+ 79+void LocaleTest::TestLongLocaleSetKeywordMoveAssign() { 80+ IcuTestErrorCode status(*this, "TestLongLocaleSetKeywordMoveAssign"); 81+ // A long base name, with an illegal keyword and copy constructor 82+ icu::Locale l("de_AAAAAAA1_AAAAAAA2_AAAAAAA3_AAAAAAA4_AAAAAAA5_AAAAAAA6_" 83+ "AAAAAAA7_AAAAAAA8_AAAAAAA9_AAAAAA10_AAAAAA11_AAAAAA12_" 84+ "AAAAAA13_AAAAAA14_AAAAAA15_AAAAAA16_AAAAAA17"); 85+ Locale l2; 86+ l.setUnicodeKeywordValue("co", "12", status); // Cause an error 87+ status.reset(); 88+ Locale l3 = std::move(l); // move assign 89+} 90+ 91 void LocaleTest::TestSetUnicodeKeywordValueNullInLongLocale() { 92 IcuTestErrorCode status(*this, "TestSetUnicodeKeywordValueNullInLongLocale"); 93 const char *exts[] = {"cf", "cu", "em", "kk", "kr", "ks", "kv", "lb", "lw", 94diff --git a/icu4c/source/test/intltest/loctest.h b/icu4c/source/test/intltest/loctest.h 95index 05be4037bd6..12a93bde53d 100644 96--- a/icu4c/source/test/intltest/loctest.h 97+++ b/icu4c/source/test/intltest/loctest.h 98@@ -156,6 +156,8 @@ class LocaleTest: public IntlTest { 99 void TestSetUnicodeKeywordValueInLongLocale(); 100 void TestSetUnicodeKeywordValueNullInLongLocale(); 101 void TestLeak21419(); 102+ void TestLongLocaleSetKeywordAssign(); 103+ void TestLongLocaleSetKeywordMoveAssign(); 104 105 private: 106 void _checklocs(const char* label, 107