(svn r22747) [1.1] -Backport from trunk:

- Fix: Some corrupted savegames could crash OpenTTD instead of showing the 'savegame corrupted' message [FS#4717] (r22737, r22736)
- Fix: Triggering NOT_REACHED when playing with a NewGRF that supplies genders/cases for a language that was not installed [FS#4718] (r22735)

src/newgrf_text.cpp

@@ -293,8 +293,9 @@ struct UnmappedChoiceList : ZeroedMemoryAllocator {
 
 		char *d = old_d;
 		if (lm == NULL && this->type != SCC_PLURAL_LIST) {
-			NOT_REACHED();
+			/* In case there is no mapping, just ignore everything but the default.
-			/* In case there is no mapping, just ignore everything but the default. */
+			 * A probable cause for this happening is when the language file has
+			 * been removed by the user and as such no mapping could be made. */
 			size_t len = strlen(this->strings[0]);
 			memcpy(d, this->strings[0], len);
 			return d + len;

src/saveload/cheat_sl.cpp

@@ -32,6 +32,8 @@ static void Load_CHTS()
 {
 	Cheat *cht = (Cheat*)&_cheats;
 	size_t count = SlGetFieldLength() / 2;
+	/* Cannot use lengthof because _cheats is of type Cheats, not Cheat */
+	if (count > sizeof(_cheats) / sizeof(Cheat)) SlErrorCorrupt("Too many cheat values");
 
 	for (uint i = 0; i < count; i++) {
 		cht[i].been_used = (SlReadByte() != 0);

src/saveload/company_sl.cpp

@@ -283,6 +283,7 @@ static void SaveLoad_PLYR_common(Company *c, CompanyProperties *cprops)
 	SlObject(&cprops->cur_economy, _company_economy_desc);
 
 	/* Write old economy entries. */
+	if (cprops->num_valid_stat_ent > lengthof(cprops->old_economy)) SlErrorCorrupt("Too many old economy entries");
 	for (i = 0; i < cprops->num_valid_stat_ent; i++) {
 		SlObject(&cprops->old_economy[i], _company_economy_desc);
 	}

src/saveload/strings_sl.cpp

@@ -15,6 +15,10 @@
 
 #include "table/strings.h"
 
+static const int NUM_OLD_STRINGS     = 512; ///< The number of custom strings stored in old savegames.
+static const int LEN_OLD_STRINGS     =  32; ///< The number of characters per string.
+static const int LEN_OLD_STRINGS_TTO =  24; ///< The number of characters per string in TTO savegames.
+
 /**
  * Remap a string ID from the old format to the new format
  * @param s StringID that requires remapping
@@ -57,10 +61,9 @@ char *CopyFromOldName(StringID id)
 	if (GB(id, 11, 5) != 15) return NULL;
 
 	if (IsSavegameVersionBefore(37)) {
-		/* Old names were 24/32 characters long, so 128 characters should be
+		/* Allow for expansion when converted to UTF-8. */
-		 * plenty to allow for expansion when converted to UTF-8. */
+		char tmp[LEN_OLD_STRINGS * MAX_CHAR_LENGTH];
-		char tmp[128];
+		uint offs = _savegame_type == SGT_TTO ? LEN_OLD_STRINGS_TTO * GB(id, 0, 8) : LEN_OLD_STRINGS * GB(id, 0, 9);
-		uint offs = _savegame_type == SGT_TTO ? 24 * GB(id, 0, 8) : 32 * GB(id, 0, 9);
 		const char *strfrom = &_old_name_array[offs];
 		char *strto = tmp;
 
@@ -92,7 +95,7 @@ char *CopyFromOldName(StringID id)
 		return strdup(tmp);
 	} else {
 		/* Name will already be in UTF-8. */
-		return strdup(&_old_name_array[32 * GB(id, 0, 9)]);
+		return strdup(&_old_name_array[LEN_OLD_STRINGS * GB(id, 0, 9)]);
 	}
 }
 
@@ -112,7 +115,7 @@ void ResetOldNames()
 void InitializeOldNames()
 {
 	free(_old_name_array);
-	_old_name_array = CallocT<char>(512 * 32); // 200 * 24 would be enough for TTO savegames
+	_old_name_array = CallocT<char>(NUM_OLD_STRINGS * LEN_OLD_STRINGS); // 200 * 24 would be enough for TTO savegames
 }
 
 static void Load_NAME()
@@ -120,7 +123,12 @@ static void Load_NAME()
 	int index;
 
 	while ((index = SlIterateArray()) != -1) {
-		SlArray(&_old_name_array[32 * index], SlGetFieldLength(), SLE_UINT8);
+		if (index >= NUM_OLD_STRINGS) SlErrorCorrupt("Invalid old name index");
+		if (SlGetFieldLength() > (uint)LEN_OLD_STRINGS) SlErrorCorrupt("Invalid old name length");
+
+		SlArray(&_old_name_array[LEN_OLD_STRINGS * index], SlGetFieldLength(), SLE_UINT8);
+		/* Make sure the old name is null terminated */
+		_old_name_array[LEN_OLD_STRINGS * index + LEN_OLD_STRINGS - 1] = '\0';
 	}
 }