Skip to content

Commit

Permalink
Fix: Potential out-of-bounds reads due to uninitialised string parame…
Browse files Browse the repository at this point in the history
…ters. (OpenTTD#13153)

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.
  • Loading branch information
PeterN authored Dec 7, 2024
1 parent f5d78f9 commit 3d73c95
Showing 1 changed file with 19 additions and 5 deletions.
24 changes: 19 additions & 5 deletions src/strings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

Expand Down Expand Up @@ -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<StringID>();
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;
Expand Down Expand Up @@ -1195,8 +1209,8 @@ static void FormatString(StringBuilder &builder, const char *str_arg, StringPara
StringID string_id = args.GetNextParameter<StringID>();
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);
Expand Down

0 comments on commit 3d73c95

Please sign in to comment.