From 14834e67cbc0dd3b7e2fdcababe8a8c81dd71c5f Mon Sep 17 00:00:00 2001 From: smatz Date: Sun, 18 Jan 2009 23:26:38 +0000 Subject: (svn r15145) -Fix: crash when one tried to load a TTO savegame -Fix (r15144): it wasn't safe at all, but the code broken code isn't needed anymore --- src/lang/english.txt | 1 + src/saveload/oldloader.cpp | 51 ++++++++++++++++++++++++++++++++-------------- 2 files changed, 37 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/lang/english.txt b/src/lang/english.txt index e85163834..01729fa0c 100644 --- a/src/lang/english.txt +++ b/src/lang/english.txt @@ -2028,6 +2028,7 @@ STR_GAME_SAVELOAD_ERROR_BROKEN_SAVEGAME :Broken savegame STR_GAME_SAVELOAD_ERROR_TOO_NEW_SAVEGAME :Savegame is made with newer version STR_GAME_SAVELOAD_ERROR_FILE_NOT_READABLE :File not readable STR_GAME_SAVELOAD_ERROR_FILE_NOT_WRITEABLE :File not writeable +STR_GAME_SAVELOAD_ERROR_DATA_INTEGRITY_CHECK_FAILED :Data integrity check failed STR_400A_LIST_OF_DRIVES_DIRECTORIES :{BLACK}List of drives, directories and saved-game files STR_400B_CURRENTLY_SELECTED_NAME :{BLACK}Currently selected name for saved-game STR_400C_DELETE_THE_CURRENTLY_SELECTED :{BLACK}Delete the currently selected saved-game diff --git a/src/saveload/oldloader.cpp b/src/saveload/oldloader.cpp index d8009878a..73aaa440b 100644 --- a/src/saveload/oldloader.cpp +++ b/src/saveload/oldloader.cpp @@ -1452,9 +1452,6 @@ static bool LoadOldMain(LoadgameState *ls) { int i; - /* The first 49 is the name of the game + checksum, skip it */ - fseek(ls->file, HEADER_SIZE, SEEK_SET); - DEBUG(oldloader, 3, "Reading main chunk..."); /* Load the biggest chunk */ _old_map3 = MallocT(OLD_MAP_SIZE * 2); @@ -1540,6 +1537,28 @@ static bool LoadOldMain(LoadgameState *ls) return true; } +/** + * Verifies the title has a valid checksum + * @param title title and checksum + * @return true iff the title is valid + * @note the title (incl. checksum) has to be at least 49 (HEADER_SIZE) bytes long! + */ +static bool VerifyOldNameChecksum(char *title) +{ + uint16 sum = 0; + for (uint i = 0; i < HEADER_SIZE - 2; i++) { + sum += title[i]; + sum = ROL(sum, 1); + } + + sum ^= 0xAAAA; // computed checksum + + uint16 sum2 = title[HEADER_SIZE - 2]; // checksum in file + SB(sum2, 8, 8, title[HEADER_SIZE - 1]); + + return sum == sum2; +} + bool LoadOldSaveGame(const char *file) { LoadgameState ls; @@ -1556,10 +1575,12 @@ bool LoadOldSaveGame(const char *file) return false; } - /* Load the main chunk */ - if (!LoadOldMain(&ls)) return false; - - fclose(ls.file); + char temp[HEADER_SIZE]; + if (fread(temp, 1, HEADER_SIZE, ls.file) != HEADER_SIZE || !VerifyOldNameChecksum(temp) || !LoadOldMain(&ls)) { + SetSaveLoadError(STR_GAME_SAVELOAD_ERROR_DATA_INTEGRITY_CHECK_FAILED); + fclose(ls.file); + return false; + } /* Some old TTD(Patch) savegames could have buoys at tile 0 * (without assigned station struct) @@ -1575,20 +1596,20 @@ bool LoadOldSaveGame(const char *file) void GetOldSaveGameName(const char *path, const char *file, char *title, const char *last) { char filename[MAX_PATH]; - char temp[HEADER_SIZE - 1]; + char temp[HEADER_SIZE]; snprintf(filename, lengthof(filename), "%s" PATHSEP "%s", path, file); FILE *f = fopen(filename, "rb"); - temp[0] = '\0'; - temp[HEADER_SIZE - 2] = '\0'; + temp[0] = '\0'; // name is nul-terminated in savegame ... if (f == NULL) return; - if (fread(temp, 1, HEADER_SIZE - 2, f) != HEADER_SIZE - 2) { - seprintf(title, last, "Corrupt file"); - } else { - seprintf(title, last, temp); - } + bool broken = (fread(temp, 1, HEADER_SIZE, f) != HEADER_SIZE || !VerifyOldNameChecksum(temp)); + + temp[HEADER_SIZE - 2] = '\0'; // ... but it's better to be sure + + if (broken) title = strecpy(title, "(broken) ", last); + title = strecpy(title, temp, last); fclose(f); } -- cgit v1.2.3-70-g09d2