From e8d328979c07e519d469779872bdc36d323d2b90 Mon Sep 17 00:00:00 2001
From: Augustin Cavalier <waddlesplash@gmail.com>
Date: Sat, 28 Oct 2023 16:36:12 -0400
Subject: [PATCH] ICUTimeConversion: Fix buffer overflows and add more error
 handling.

 * Declare databridge buffer lengths in LocaleBackend.
 * Use strcpy instead of strlcpy when writing to databridge buffers
   (this is the first fix for #18598.)
 * Check for overflows and error out when they happen.
 * Verify that ICU actually knows the timezone in question
   and fall back to GMT if it does not (this would also fix that crash.)

Fixes #18598.
---
 .../private/libroot/locale/LocaleBackend.h    |  6 ++--
 .../libroot/add-ons/icu/ICUTimeConversion.cpp | 30 ++++++++++++-------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/headers/private/libroot/locale/LocaleBackend.h b/headers/private/libroot/locale/LocaleBackend.h
index 272bbf77dc..20c06ec0d3 100644
--- a/headers/private/libroot/locale/LocaleBackend.h
+++ b/headers/private/libroot/locale/LocaleBackend.h
@@ -102,12 +102,14 @@ struct LocaleTimeDataBridge {
 
 
 struct TimeConversionDataBridge {
+	static const int32 		kTZNameLength = 64;
+
 private:
 	int						localDaylight;
 	long					localTimezone;
 	char*					localTZName[2];
-	char					localTZName0[64];
-	char					localTZName1[64];
+	char					localTZName0[kTZNameLength];
+	char					localTZName1[kTZNameLength];
 
 public:
 	int*					addrOfDaylight;
diff --git a/src/system/libroot/add-ons/icu/ICUTimeConversion.cpp b/src/system/libroot/add-ons/icu/ICUTimeConversion.cpp
index c076cf7d50..801db23005 100644
--- a/src/system/libroot/add-ons/icu/ICUTimeConversion.cpp
+++ b/src/system/libroot/add-ons/icu/ICUTimeConversion.cpp
@@ -113,6 +113,10 @@ done:
 	if (fTimeZone == NULL)
 		return B_NO_MEMORY;
 
+	UnicodeString temp;
+	if (fTimeZone->getID(temp) == UCAL_UNKNOWN_ZONE_ID)
+		goto error;
+
 	if (offsetHasBeenSet) {
 		fTimeZone->setRawOffset(*fDataBridge->addrOfTimezone * -1 * 1000);
 	} else {
@@ -121,14 +125,8 @@ done:
 		UDate nowMillis = 1000 * (UDate)time(NULL);
 		UErrorCode icuStatus = U_ZERO_ERROR;
 		fTimeZone->getOffset(nowMillis, FALSE, rawOffset, dstOffset, icuStatus);
-		if (!U_SUCCESS(icuStatus)) {
-			*fDataBridge->addrOfTimezone = 0;
-			*fDataBridge->addrOfDaylight = false;
-			strcpy(fDataBridge->addrOfTZName[0], "GMT");
-			strcpy(fDataBridge->addrOfTZName[1], "GMT");
-
-			return B_ERROR;
-		}
+		if (!U_SUCCESS(icuStatus))
+			goto error;
 		*fDataBridge->addrOfTimezone = -1 * (rawOffset + dstOffset) / 1000;
 			// we want seconds, not the ms that ICU gives us
 	}
@@ -137,15 +135,19 @@ done:
 
 	for (int i = 0; i < 2; ++i) {
 		if (tz != NULL && *tz != ':' && i == 0) {
-			strcpy(fDataBridge->addrOfTZName[0], fTimeZoneID);
+			strlcpy(fDataBridge->addrOfTZName[0], fTimeZoneID,
+				fDataBridge->kTZNameLength);
 		} else {
 			UnicodeString icuString;
 			fTimeZone->getDisplayName(i == 1, TimeZone::SHORT,
 				fTimeData.ICULocaleForStrings(), icuString);
 			CheckedArrayByteSink byteSink(fDataBridge->addrOfTZName[i],
-				sizeof(fTimeZoneID));
+				fDataBridge->kTZNameLength);
 			icuString.toUTF8(byteSink);
 
+			if (byteSink.Overflowed())
+				goto error;
+
 			// make sure to canonicalize "GMT+00:00" to just "GMT"
 			if (strcmp(fDataBridge->addrOfTZName[i], "GMT+00:00") == 0)
 				fDataBridge->addrOfTZName[i][3] = '\0';
@@ -153,6 +155,14 @@ done:
 	}
 
 	return B_OK;
+
+error:
+	*fDataBridge->addrOfTimezone = 0;
+	*fDataBridge->addrOfDaylight = false;
+	strcpy(fDataBridge->addrOfTZName[0], "GMT");
+	strcpy(fDataBridge->addrOfTZName[1], "GMT");
+
+	return B_ERROR;
 }