Fix #7553: check bounds when loading strings (#7554)

2 files changed, 15 insertions, 11 deletions

diff --git a/src/strgen/strgen.h b/src/strgen/strgen.h
index fd527203b..69b8732f1 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 804ce6498..84fc3537b 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<LangString *>(max_strings);
-	this->hash_heads = CallocT<uint16>(max_strings);
+	this->hash_heads = CallocT<size_t>(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);
+	}
 }
 
 /**