From cfffa2b83bebbedc6981397641b7eb516be9d9e7 Mon Sep 17 00:00:00 2001 From: Frank Yung-Fong Tang Date: Wed, 14 Jul 2021 00:47:01 +0000 Subject: [PATCH] ICU-21676 Fix strcpy buffer override in ultag_parse See #1767 --- icu4c/source/common/uloc_tag.cpp | 6 +++++- icu4c/source/test/intltest/loctest.cpp | 21 +++++++++++++++++++++ icu4c/source/test/intltest/loctest.h | 1 + 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/icu4c/source/common/uloc_tag.cpp b/icu4c/source/common/uloc_tag.cpp index dabe6e44825..1b91da7a4dd 100644 --- a/icu4c/source/common/uloc_tag.cpp +++ b/icu4c/source/common/uloc_tag.cpp @@ -2089,6 +2089,7 @@ ultag_parse(const char* tag, int32_t tagLen, int32_t* parsedLen, UErrorCode* sta legacyLen = checkLegacyLen; /* back up for output parsedLen */ int32_t replacementLen = static_cast(uprv_strlen(LEGACY[i+1])); newTagLength = replacementLen + tagLen - checkLegacyLen; + int32_t oldTagLength = tagLen; if (tagLen < newTagLength) { uprv_free(tagBuf); tagBuf = (char*)uprv_malloc(newTagLength + 1); @@ -2102,7 +2103,10 @@ ultag_parse(const char* tag, int32_t tagLen, int32_t* parsedLen, UErrorCode* sta parsedLenDelta = checkLegacyLen - replacementLen; uprv_strcpy(t->buf, LEGACY[i + 1]); if (checkLegacyLen != tagLen) { - uprv_strcpy(t->buf + replacementLen, tag + checkLegacyLen); + uprv_memcpy(t->buf + replacementLen, tag + checkLegacyLen, + oldTagLength - checkLegacyLen); + // NUL-terminate after memcpy(). + t->buf[replacementLen + oldTagLength - checkLegacyLen] = 0; } break; } diff --git a/icu4c/source/test/intltest/loctest.cpp b/icu4c/source/test/intltest/loctest.cpp index fc156f3ca3a..277197a6941 100644 --- a/icu4c/source/test/intltest/loctest.cpp +++ b/icu4c/source/test/intltest/loctest.cpp @@ -265,6 +265,7 @@ void LocaleTest::runIndexedTest( int32_t index, UBool exec, const char* &name, c TESTCASE_AUTO(TestKnownCanonicalizedListCorrect); TESTCASE_AUTO(TestConstructorAcceptsBCP47); TESTCASE_AUTO(TestForLanguageTag); + TESTCASE_AUTO(TestForLanguageTagLegacyTagBug21676); TESTCASE_AUTO(TestToLanguageTag); TESTCASE_AUTO(TestToLanguageTagOmitTrue); TESTCASE_AUTO(TestMoveAssign); @@ -5677,6 +5678,26 @@ void LocaleTest::TestForLanguageTag() { } } +void LocaleTest::TestForLanguageTagLegacyTagBug21676() { + IcuTestErrorCode status(*this, "TestForLanguageTagLegacyTagBug21676()"); + std::string tag( + "i-enochian-1nochian-129-515VNTR-64863775-X3il6-110Y101-29-515VNTR-" + "64863775-153zu-u-Y4-H0-t6-X3-u6-110Y101-X"); + std::string input(tag); + input += "EXTRA MEMORY AFTER NON-NULL TERMINATED STRING"; + StringPiece stringp(input.c_str(), tag.length()); + std::string name = Locale::forLanguageTag(stringp, status).getName(); + std::string expected( + "@x=i-enochian-1nochian-129-515vntr-64863775-x3il6-110y101-29-515vntr-" + "64863775-153zu-u-y4-h0-t6-x3-u6-110y101-x"); + if (name != expected) { + errcheckln( + status, + "FAIL: forLanguageTag('%s', \n%d).getName() should be \n'%s' but got %s", + tag.c_str(), tag.length(), expected.c_str(), name.c_str()); + } +} + void LocaleTest::TestToLanguageTag() { IcuTestErrorCode status(*this, "TestToLanguageTag()"); diff --git a/icu4c/source/test/intltest/loctest.h b/icu4c/source/test/intltest/loctest.h index 4a856c62239..2c91520dd17 100644 --- a/icu4c/source/test/intltest/loctest.h +++ b/icu4c/source/test/intltest/loctest.h @@ -133,6 +133,7 @@ public: void TestAddLikelyAndMinimizeSubtags(); void TestForLanguageTag(); + void TestForLanguageTagLegacyTagBug21676(); void TestToLanguageTag(); void TestToLanguageTagOmitTrue();