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
This commit is contained in:
Ben Harris
2023-01-07 19:32:08 +00:00
parent e5717d1ba2
commit b3d4a41979

View File

@ -2418,7 +2418,12 @@ static const char *midend_deserialise_internal(
ret = "No state count provided in save file"; ret = "No state count provided in save file";
goto cleanup; goto cleanup;
} }
if (data.statepos < 0) {
ret = "No game position provided in save file";
goto cleanup;
}
gotstates++; gotstates++;
assert(gotstates < data.nstates);
if (!strcmp(key, "MOVE")) if (!strcmp(key, "MOVE"))
data.states[gotstates].movetype = MOVE; data.states[gotstates].movetype = MOVE;
else if (!strcmp(key, "SOLVE")) else if (!strcmp(key, "SOLVE"))