Combined backport of: ~~~ From 5c711e36e8e6a9810bb689ace16f698d1d3ab9ce Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Thu, 28 Sep 2017 01:00:43 +0000 Subject: [PATCH] ICU-13327 Fixing buffer overflow in NumberingSystem constructor. X-SVN-Rev: 40494 --- icu4c/source/i18n/numsys.cpp | 7 +++++++ icu4c/source/test/intltest/numfmtst.cpp | 22 ++++++++++++++++++++++ icu4c/source/test/intltest/numfmtst.h | 1 + 3 files changed, 30 insertions(+) ~~~ ~~~ From 1c794eb111bb9c146375038dd66806aaa26f35ce Mon Sep 17 00:00:00 2001 From: Andy Heninger Date: Wed, 11 Oct 2017 22:22:45 +0000 Subject: [PATCH] ICU-13394 nul-terminated buffer handling fixed from Chromium. X-SVN-Rev: 40615 --- icu4c/source/common/locdispnames.cpp | 2 ++ icu4c/source/common/locdspnm.cpp | 3 ++- icu4c/source/common/ucurr.cpp | 1 + icu4c/source/i18n/ucol_sit.cpp | 11 +++++++++-- 4 files changed, 14 insertions(+), 3 deletions(-) ~~~ --- source/common/locdispnames.cpp +++ source/common/locdispnames.cpp @@ -821,6 +821,8 @@ uloc_getDisplayKeywordValue( const char* locale, /* get the keyword value */ keywordValue[0]=0; keywordValueLen = uloc_getKeywordValue(locale, keyword, keywordValue, capacity, status); + if (*status == U_STRING_NOT_TERMINATED_WARNING) + *status = U_BUFFER_OVERFLOW_ERROR; /* * if the keyword is equal to currency .. then to get the display name --- source/common/locdspnm.cpp +++ source/common/locdspnm.cpp @@ -635,8 +635,9 @@ LocaleDisplayNamesImpl::localeDisplayName(const Locale& locale, char value[ULOC_KEYWORD_AND_VALUES_CAPACITY]; // sigh, no ULOC_VALUE_CAPACITY const char* key; while ((key = e->next((int32_t *)0, status)) != NULL) { + value[0] = 0; locale.getKeywordValue(key, value, ULOC_KEYWORD_AND_VALUES_CAPACITY, status); - if (U_FAILURE(status)) { + if (U_FAILURE(status) || status == U_STRING_NOT_TERMINATED_WARNING) { return result; } keyDisplayName(key, temp, TRUE); --- source/common/ucurr.cpp +++ source/common/ucurr.cpp @@ -2215,6 +2215,7 @@ ucurr_countCurrencies(const char* locale, UErrorCode localStatus = U_ZERO_ERROR; char id[ULOC_FULLNAME_CAPACITY]; uloc_getKeywordValue(locale, "currency", id, ULOC_FULLNAME_CAPACITY, &localStatus); + // get country or country_variant in `id' /*uint32_t variantType =*/ idForLocale(locale, id, sizeof(id), ec); --- source/i18n/numsys.cpp +++ source/i18n/numsys.cpp @@ -25,6 +25,7 @@ #include "unicode/schriter.h" #include "unicode/numsys.h" #include "cstring.h" +#include "uassert.h" #include "uresimp.h" #include "numsys_impl.h" @@ -115,7 +116,13 @@ NumberingSystem::createInstance(const Locale & inLocale, UErrorCode& status) { UBool usingFallback = FALSE; char buffer[ULOC_KEYWORDS_CAPACITY]; int32_t count = inLocale.getKeywordValue("numbers",buffer, sizeof(buffer),status); + if (U_FAILURE(status) || status == U_STRING_NOT_TERMINATED_WARNING) { + // the "numbers" keyword exceeds ULOC_KEYWORDS_CAPACITY; ignore and use default. + count = 0; + status = U_ZERO_ERROR; + } if ( count > 0 ) { // @numbers keyword was specified in the locale + U_ASSERT(count < ULOC_KEYWORDS_CAPACITY); buffer[count] = '\0'; // Make sure it is null terminated. if ( !uprv_strcmp(buffer,gDefault) || !uprv_strcmp(buffer,gNative) || !uprv_strcmp(buffer,gTraditional) || !uprv_strcmp(buffer,gFinance)) { --- source/i18n/ucol_sit.cpp +++ source/i18n/ucol_sit.cpp @@ -465,8 +465,15 @@ ucol_prepareShortStringOpen( const char *definition, UResourceBundle *collElem = NULL; char keyBuffer[256]; // if there is a keyword, we pick it up and try to get elements - if(!uloc_getKeywordValue(buffer, "collation", keyBuffer, 256, status)) { - // no keyword. we try to find the default setting, which will give us the keyword value + int32_t keyLen = uloc_getKeywordValue(buffer, "collation", keyBuffer, sizeof(keyBuffer), status); + // Treat too long a value as no keyword. + if(keyLen >= (int32_t)sizeof(keyBuffer)) { + keyLen = 0; + *status = U_ZERO_ERROR; + } + if(keyLen == 0) { + // no keyword + // we try to find the default setting, which will give us the keyword value UResourceBundle *defaultColl = ures_getByKeyWithFallback(collations, "default", NULL, status); if(U_SUCCESS(*status)) { int32_t defaultKeyLen = 0; --- source/test/intltest/numfmtst.cpp +++ source/test/intltest/numfmtst.cpp @@ -581,6 +581,7 @@ void NumberFormatTest::runIndexedTest( int32_t index, UBool exec, const char* &n TESTCASE_AUTO(Test11475_signRecognition); TESTCASE_AUTO(Test11640_getAffixes); TESTCASE_AUTO(Test11649_toPatternWithMultiCurrency); + TESTCASE_AUTO(Test13327_numberingSystemBufferOverflow); TESTCASE_AUTO_END; } @@ -8717,6 +8718,27 @@ void NumberFormatTest::Test11649_toPatternWithMultiCurrency() { assertEquals("", "US dollars 12.34", fmt2.format(12.34, appendTo)); } +void NumberFormatTest::Test13327_numberingSystemBufferOverflow() { + UErrorCode status; + for (int runId = 0; runId < 2; runId++) { + // Construct a locale string with a very long "numbers" value. + // The first time, make the value length exactly equal to ULOC_KEYWORDS_CAPACITY. + // The second time, make it exceed ULOC_KEYWORDS_CAPACITY. + int extraLength = (runId == 0) ? 0 : 5; + + CharString localeId("en@numbers=", status); + for (int i = 0; i < ULOC_KEYWORDS_CAPACITY + extraLength; i++) { + localeId.append('x', status); + } + assertSuccess("Constructing locale string", status); + Locale locale(localeId.data()); + + NumberingSystem* ns = NumberingSystem::createInstance(locale, status); + assertFalse("Should not be null", ns == nullptr); + assertSuccess("Should create with no error", status); + } +} + void NumberFormatTest::verifyFieldPositionIterator( NumberFormatTest_Attributes *expected, FieldPositionIterator &iter) { --- source/test/intltest/numfmtst.h +++ source/test/intltest/numfmtst.h @@ -215,6 +215,7 @@ class NumberFormatTest: public CalendarTimeZoneTest { void Test11475_signRecognition(); void Test11640_getAffixes(); void Test11649_toPatternWithMultiCurrency(); + void Test13327_numberingSystemBufferOverflow(); void checkExceptionIssue11735();