From cef9a76c3ffaaf42bd9b85ff41afb0d9d45d05bc Mon Sep 17 00:00:00 2001 From: glx22 Date: Wed, 1 May 2019 19:12:37 +0200 Subject: [PATCH] Fix #7553: check bounds when loading strings (#7554) --- src/stdafx.h | 14 ++++++++------ src/strgen/strgen.h | 10 +++++----- src/strgen/strgen_base.cpp | 16 ++++++++++------ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/stdafx.h b/src/stdafx.h index f85d68aa04..123dffa138 100644 --- a/src/stdafx.h +++ b/src/stdafx.h @@ -284,13 +284,15 @@ /* MSVCRT of course has to have a different syntax for long long *sigh* */ #if defined(_MSC_VER) || defined(__MINGW32__) - #define OTTD_PRINTF64 "%I64d" - #define OTTD_PRINTFHEX64 "%I64x" - #define PRINTF_SIZE "%Iu" +# define OTTD_PRINTF64 "%I64d" +# define OTTD_PRINTFHEX64 "%I64x" +# define PRINTF_SIZE "%Iu" +# define PRINTF_SIZEX "%IX" #else - #define OTTD_PRINTF64 "%lld" - #define OTTD_PRINTFHEX64 "%llx" - #define PRINTF_SIZE "%zu" +# define OTTD_PRINTF64 "%lld" +# define OTTD_PRINTFHEX64 "%llx" +# define PRINTF_SIZE "%zu" +# define PRINTF_SIZEX "%zX" #endif typedef unsigned char byte; diff --git a/src/strgen/strgen.h b/src/strgen/strgen.h index fd527203b3..69b8732f17 100644 --- a/src/strgen/strgen.h +++ b/src/strgen/strgen.h @@ -29,12 +29,12 @@ struct LangString { char *name; ///< Name of the string. char *english; ///< English text. char *translated; ///< Translated text. - uint16 hash_next; ///< Next hash entry. - uint16 index; ///< The index in the language file. + size_t hash_next; ///< Next hash entry. + size_t index; ///< The index in the language file. int line; ///< Line of string in source-file. Case *translated_case; ///< Cases of the translation. - LangString(const char *name, const char *english, int index, int line); + LangString(const char *name, const char *english, size_t index, int line); ~LangString(); void FreeTranslation(); }; @@ -42,10 +42,10 @@ struct LangString { /** Information about the currently known strings. */ struct StringData { LangString **strings; ///< Array of all known strings. - uint16 *hash_heads; ///< Hash table for the strings. + size_t *hash_heads; ///< Hash table for the strings. size_t tabs; ///< The number of 'tabs' of strings. size_t max_strings; ///< The maximum number of strings. - int next_string_id; ///< The next string ID to allocate. + size_t next_string_id;///< The next string ID to allocate. StringData(size_t tabs); ~StringData(); diff --git a/src/strgen/strgen_base.cpp b/src/strgen/strgen_base.cpp index 804ce6498c..84fc3537be 100644 --- a/src/strgen/strgen_base.cpp +++ b/src/strgen/strgen_base.cpp @@ -58,7 +58,7 @@ Case::~Case() * @param index The index in the string table. * @param line The line this string was found on. */ -LangString::LangString(const char *name, const char *english, int index, int line) : +LangString::LangString(const char *name, const char *english, size_t index, int line) : name(stredup(name)), english(stredup(english)), translated(nullptr), hash_next(0), index(index), line(line), translated_case(nullptr) { @@ -90,7 +90,7 @@ void LangString::FreeTranslation() StringData::StringData(size_t tabs) : tabs(tabs), max_strings(tabs * TAB_SIZE) { this->strings = CallocT(max_strings); - this->hash_heads = CallocT(max_strings); + this->hash_heads = CallocT(max_strings); this->next_string_id = 0; } @@ -144,9 +144,9 @@ void StringData::Add(const char *s, LangString *ls) */ LangString *StringData::Find(const char *s) { - int idx = this->hash_heads[this->HashStr(s)]; + size_t idx = this->hash_heads[this->HashStr(s)]; - while (--idx >= 0) { + while (idx-- > 0) { LangString *ls = this->strings[idx]; if (strcmp(ls->name, s) == 0) return ls; @@ -764,7 +764,7 @@ void StringReader::HandleString(char *str) } if (this->data.strings[this->data.next_string_id] != nullptr) { - strgen_error("String ID 0x%X for '%s' already in use by '%s'", this->data.next_string_id, str, this->data.strings[this->data.next_string_id]->name); + strgen_error("String ID 0x" PRINTF_SIZEX " for '%s' already in use by '%s'", this->data.next_string_id, str, this->data.strings[this->data.next_string_id]->name); return; } @@ -830,11 +830,15 @@ void StringReader::ParseFile() strecpy(_lang.digit_decimal_separator, ".", lastof(_lang.digit_decimal_separator)); _cur_line = 1; - while (this->ReadLine(buf, lastof(buf)) != nullptr) { + while (this->data.next_string_id < this->data.max_strings && this->ReadLine(buf, lastof(buf)) != nullptr) { rstrip(buf); this->HandleString(buf); _cur_line++; } + + if (this->data.next_string_id == this->data.max_strings) { + strgen_error("Too many strings, maximum allowed is " PRINTF_SIZE, this->data.max_strings); + } } /**