From 20d43db4a7c3505454d3b294a1556c526aca3df6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Tue, 28 Mar 2023 17:10:59 +0200 Subject: [PATCH] Fix invalid indices to user strings (#19738) C++ treats `%` (mod) operation differently to common knowledge. While `1 % 4` yields expected `1`, `-1 % 4` yields `-1` instead of expected `3`. This can lead to user string IDs with low values (less than `USER_STRING_START` or 0x8000) to become negative, resulting in incorrect reads from user strings array. Instead of trying to massage the value less `USER_STRING_START` into expected range, I observed the original value (of uint16_t type) would be congruent even if we didn't subtract, as both modulus and subtrahent are sufficiently large powers of two. This is _possibly_ the reason for issues such as https://github.com/OpenRCT2/OpenRCT2/issues/19240 where crash occured during park load. This can be trivially reproduced by loading "Prehistoric - After the Asteroid" from "Time Twister". --- src/openrct2/localisation/Localisation.h | 1 - src/openrct2/rct1/S4Importer.cpp | 2 +- src/openrct2/rct2/S6Importer.cpp | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/openrct2/localisation/Localisation.h b/src/openrct2/localisation/Localisation.h index 447f543d5b..3a250d3649 100644 --- a/src/openrct2/localisation/Localisation.h +++ b/src/openrct2/localisation/Localisation.h @@ -45,7 +45,6 @@ bool IsUserStringID(StringId stringId); #define MAX_USER_STRINGS 1024 #define USER_STRING_MAX_LENGTH 32 -#define USER_STRING_START 0x8000 #define USER_STRING_END 0x8FFF #define REAL_NAME_START 0xA000 #define REAL_NAME_END 0xDFFF diff --git a/src/openrct2/rct1/S4Importer.cpp b/src/openrct2/rct1/S4Importer.cpp index efb443fba2..f8951077cc 100644 --- a/src/openrct2/rct1/S4Importer.cpp +++ b/src/openrct2/rct1/S4Importer.cpp @@ -2470,7 +2470,7 @@ namespace RCT1 std::string GetUserString(StringId stringId) { - const auto originalString = _s4.StringTable[(stringId - USER_STRING_START) % 1024]; + const auto originalString = _s4.StringTable[stringId % 1024]; auto originalStringView = std::string_view( originalString, RCT2::GetRCT2StringBufferLen(originalString, USER_STRING_MAX_LENGTH)); auto asUtf8 = RCT2StringToUTF8(originalStringView, RCT2LanguageId::EnglishUK); diff --git a/src/openrct2/rct2/S6Importer.cpp b/src/openrct2/rct2/S6Importer.cpp index aad22cf0c1..e47482ae28 100644 --- a/src/openrct2/rct2/S6Importer.cpp +++ b/src/openrct2/rct2/S6Importer.cpp @@ -1858,7 +1858,7 @@ namespace RCT2 std::string GetUserString(StringId stringId) { - const auto originalString = _s6.CustomStrings[(stringId - USER_STRING_START) % 1024]; + const auto originalString = _s6.CustomStrings[stringId % 1024]; auto originalStringView = std::string_view( originalString, GetRCT2StringBufferLen(originalString, USER_STRING_MAX_LENGTH)); auto asUtf8 = RCT2StringToUTF8(originalStringView, RCT2LanguageId::EnglishUK);