From ecc2e180bd09e72e19edd5ae0287dd0d77d76b71 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Sat, 7 Dec 2024 01:01:40 +0000 Subject: [PATCH] Fix: Potential out-of-bounds reads due to uninitialised string parameters. If string parameters are not set correctly, FormatString can read out of bounds and crash the game. This does not fix the root cause, just a nasty symptom. --- src/strings.cpp | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/strings.cpp b/src/strings.cpp index 6514a50a42..a5c3e90005 100644 --- a/src/strings.cpp +++ b/src/strings.cpp @@ -238,7 +238,11 @@ const char *GetStringPtr(StringID string) /* 0xD0xx and 0xD4xx IDs have been converted earlier. */ case TEXT_TAB_OLD_NEWGRF: NOT_REACHED(); case TEXT_TAB_NEWGRF_START: return GetGRFStringPtr(GetStringIndex(string)); - default: return _langpack.offsets[_langpack.langtab_start[GetStringTab(string)] + GetStringIndex(string)]; + default: { + const uint offset = _langpack.langtab_start[GetStringTab(string)] + GetStringIndex(string); + if (offset < _langpack.offsets.size()) return _langpack.offsets[offset]; + return nullptr; + } } } @@ -1063,13 +1067,23 @@ static void FormatString(StringBuilder &builder, const char *str_arg, StringPara case SCC_NEWGRF_STRINL: { StringID substr = Utf8Consume(&str); - str_stack.push(GetStringPtr(substr)); + const char *ptr = GetStringPtr(substr); + if (ptr == nullptr) { + builder += "(invalid NewGRF string)"; + } else { + str_stack.push(ptr); + } break; } case SCC_NEWGRF_PRINT_WORD_STRING_ID: { StringID substr = args.GetNextParameter(); - str_stack.push(GetStringPtr(substr)); + const char *ptr = GetStringPtr(substr); + if (ptr == nullptr) { + builder += "(invalid NewGRF string)"; + } else { + str_stack.push(ptr); + } case_index = next_substr_case_index; next_substr_case_index = 0; break; @@ -1195,8 +1209,8 @@ static void FormatString(StringBuilder &builder, const char *str_arg, StringPara StringID string_id = args.GetNextParameter(); if (game_script && GetStringTab(string_id) != TEXT_TAB_GAMESCRIPT_START) break; uint size = b - SCC_STRING1 + 1; - if (game_script && size > args.GetDataLeft()) { - builder += "(too many parameters)"; + if (size > args.GetDataLeft()) { + builder += "(consumed too many parameters)"; } else { StringParameters sub_args(args, game_script ? args.GetDataLeft() : size); GetStringWithArgs(builder, string_id, sub_args, next_substr_case_index, game_script);