From b3d4a4197954c21ac78b68c58dff8f84fe743ea2 Mon Sep 17 00:00:00 2001 From: Ben Harris Date: Sat, 7 Jan 2023 19:32:08 +0000 Subject: [PATCH] Don't load too many states just because there's no STATEPOS If we start seeing state records in a save file (MOVE, SOLVE, or RESTART), we should already have seen STATEPOS, so emit an error if not. This avoids the situation where we overrun the end of the state array because we're continuing loading states in the hope a STATEPOS will come along. I've also added an assertion that we're not overrunning the state array for added paranoia. An earlier version of this fix just removed the test for data.statepos at the head of the loop, but that's wrong for a file that only has the initial state. This bug can be demonstrated by building Bridges with AddressSanitizer and loading this save file: SAVEFILE:41:Simon Tatham's Portable Puzzle Collection VERSION :1:1 GAME :7:Bridges PARAMS :13:7x7i30e10m2d0 CPARAMS :13:7x7i30e10m2d0 DESC :24:a4b4a1g1a2a8a4a4m2b2b3e3 NSTATES :1:2 MOVE :10:L1,0,4,0,1 MOVE :10:L1,0,4,0,2 --- midend.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/midend.c b/midend.c index 27c015b..6c1a599 100644 --- a/midend.c +++ b/midend.c @@ -2418,7 +2418,12 @@ static const char *midend_deserialise_internal( ret = "No state count provided in save file"; goto cleanup; } + if (data.statepos < 0) { + ret = "No game position provided in save file"; + goto cleanup; + } gotstates++; + assert(gotstates < data.nstates); if (!strcmp(key, "MOVE")) data.states[gotstates].movetype = MOVE; else if (!strcmp(key, "SOLVE"))