ref: b3d4a4197954c21ac78b68c58dff8f84fe743ea2
parent: e5717d1ba2184eb6e38b4e2a9d29dc4704aeef30
author: Ben Harris <bjh21@bjh21.me.uk>
date: Sat Jan 7 14:32:08 EST 2023
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
--- a/midend.c
+++ b/midend.c
@@ -2418,7 +2418,12 @@
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"))