From: Jeff Genovy <29107334+jefgen@users.noreply.github.com>
Date: Mon, 2 Mar 2020 00:31:55 +0000 (-0800)
Subject: ICU-21004 Fix buffer over-read in ucal_open
X-Git-Tag: release-67-rc~78
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bd08ba2c5b1c875cab36d2f5a9aec6216559b8fa;p=icu

ICU-21004 Fix buffer over-read in ucal_open

The issue shows under valgrind or as an Address Sanitizer failure.
---

diff --git a/icu4c/source/i18n/ucal.cpp b/icu4c/source/i18n/ucal.cpp
index 927b2d36979..67c51aea271 100644
--- a/icu4c/source/i18n/ucal.cpp
+++ b/icu4c/source/i18n/ucal.cpp
@@ -154,25 +154,31 @@ ucal_open(  const UChar*  zoneID,
             UCalendarType caltype,
             UErrorCode*   status)
 {
-
-  if(U_FAILURE(*status)) return 0;
+  if (U_FAILURE(*status)) {
+      return nullptr;
+  }
   
-  LocalPointer<TimeZone> zone( (zoneID==NULL) ? TimeZone::createDefault() 
+  LocalPointer<TimeZone> zone( (zoneID==nullptr) ? TimeZone::createDefault() 
       : _createTimeZone(zoneID, len, status), *status);
 
   if (U_FAILURE(*status)) {
-      return NULL;
+      return nullptr;
   }
 
   if ( caltype == UCAL_GREGORIAN ) {
-      char  localeBuf[ULOC_LOCALE_IDENTIFIER_CAPACITY];
-      if ( locale == NULL ) {
+      char localeBuf[ULOC_LOCALE_IDENTIFIER_CAPACITY];
+      if ( locale == nullptr ) {
           locale = uloc_getDefault();
       }
-      uprv_strncpy(localeBuf, locale, ULOC_LOCALE_IDENTIFIER_CAPACITY);
+      int32_t localeLength = static_cast<int32_t>(uprv_strlen(locale));
+      if (localeLength >= ULOC_LOCALE_IDENTIFIER_CAPACITY) {
+          *status = U_ILLEGAL_ARGUMENT_ERROR;
+          return nullptr;
+      }
+      uprv_strcpy(localeBuf, locale);
       uloc_setKeywordValue("calendar", "gregorian", localeBuf, ULOC_LOCALE_IDENTIFIER_CAPACITY, status);
       if (U_FAILURE(*status)) {
-          return NULL;
+          return nullptr;
       }
       return (UCalendar*)Calendar::createInstance(zone.orphan(), Locale(localeBuf), *status);
   }
@@ -182,8 +188,9 @@ ucal_open(  const UChar*  zoneID,
 U_CAPI void U_EXPORT2
 ucal_close(UCalendar *cal)
 {
-
-  delete (Calendar*) cal;
+    if (cal != nullptr) {
+        delete (Calendar*) cal;
+    }
 }
 
 U_CAPI UCalendar* U_EXPORT2 
diff --git a/icu4c/source/test/cintltst/ccaltst.c b/icu4c/source/test/cintltst/ccaltst.c
index 44cf712e0fe..c15d2771e5f 100644
--- a/icu4c/source/test/cintltst/ccaltst.c
+++ b/icu4c/source/test/cintltst/ccaltst.c
@@ -37,10 +37,10 @@ void TestGregorianChange(void);
 void TestFieldDifference(void);
 void TestAddRollEra0AndEraBounds(void);
 void TestGetTZTransition(void);
-
 void TestGetWindowsTimeZoneID(void);
 void TestGetTimeZoneIDByWindowsID(void);
 void TestJpnCalAddSetNextEra(void);
+void TestUcalOpenBufferRead(void);
 
 void addCalTest(TestNode** root);
 
@@ -64,6 +64,7 @@ void addCalTest(TestNode** root)
     addTest(root, &TestGetWindowsTimeZoneID, "tsformat/ccaltst/TestGetWindowsTimeZoneID");
     addTest(root, &TestGetTimeZoneIDByWindowsID, "tsformat/ccaltst/TestGetTimeZoneIDByWindowsID");
     addTest(root, &TestJpnCalAddSetNextEra, "tsformat/ccaltst/TestJpnCalAddSetNextEra");
+    addTest(root, &TestUcalOpenBufferRead, "tsformat/ccaltst/TestUcalOpenBufferRead");
 }
 
 /* "GMT" */
@@ -2543,4 +2544,13 @@ void TestJpnCalAddSetNextEra() {
     }
 }
 
+void TestUcalOpenBufferRead() {
+    // ICU-21004: The issue shows under valgrind or as an Address Sanitizer failure.
+    UErrorCode status = U_ZERO_ERROR;
+    // string length: 157 + 1 + 100 = 258
+    const char *localeID = "x-privatebutreallylongtagfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobar-foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoorbarfoobarfoo";
+    UCalendar *cal = ucal_open(NULL, 0, localeID, UCAL_GREGORIAN, &status);
+    ucal_close(cal);
+}
+
 #endif /* #if !UCONFIG_NO_FORMATTING */