summaryrefslogtreecommitdiff
path: root/src/gamelist.c
diff options
context:
space:
mode:
authorScott Gasch <[email protected]>2026-10-25 11:05:10 -0700
committerScott Gasch <[email protected]>2026-10-25 11:05:10 -0700
commit455accb4cbb1777cb553b6d75136521c6b8d1c01 (patch)
tree3f526bf50b58829453799bd876ff2d4e86201338 /src/gamelist.c
parentfc873933e21d9188f2b5a9d13764f00b6ce68ec9 (diff)
Fix SIGSEGV in GetNthOfficialMoveRecord on an exact-length index
GetNthOfficialMoveRecord's end-of-list check only tested whether its decrementing counter n reached 0, not whether the walk pointer p had also reached the move list's own sentinel head at the same moment. Requesting index == the list's real length (exactly what every caller's "walk until NULL" loop does as its final probe) hit both conditions simultaneously, so the function treated the sentinel's own DLIST_ENTRY as a GAME_MOVE via CONTAINING_STRUCT and handed back a pointer to g_GameData.sMoveList's own links field reinterpreted as a move record -- q->mv.uMove read as nonzero garbage (never actually 0, so the "is this real" check downstream never caught it), and q->szMoveInSan was effectively an arbitrary pointer. Reported as: play a few moves (e.g. e4 e5 Nc3), run "dump moves" -> crash. Confirmed via lldb: SIGSEGV in vsnprintf/strlen, called from Trace() in DumpGameList (gamelist.c:575) handed that garbage szMoveInSan pointer. book.c's two GetNthOfficialMoveRecord loops (opening-book move matching) have the identical exposure and are fixed by the same change at the source. Fix: also require p != &g_GameData.sMoveList before treating it as a real node. Verified: "dump moves"/"dump pgn" after several moves (both odd- and even-length move lists, so the off-by-one's exact trigger condition is covered on both sides) no longer crash, and a clean DEBUG=1 TEST=1 self-test run shows no regressions. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Diffstat (limited to 'src/gamelist.c')
-rw-r--r--src/gamelist.c17
1 files changed, 14 insertions, 3 deletions
diff --git a/src/gamelist.c b/src/gamelist.c
index 0a0bdd1..a2752a9 100644
--- a/src/gamelist.c
+++ b/src/gamelist.c
@@ -419,15 +419,26 @@ Return value:
{
DLIST_ENTRY *p = g_GameData.sMoveList.pFlink;
GAME_MOVE *q;
-
+
while((p != &(g_GameData.sMoveList)) &&
(n != 0))
{
p = p->pFlink;
n--;
}
-
- if (n == 0)
+
+ // n hitting 0 on the SAME iteration p reaches the list's own
+ // sentinel head (i.e. n == the list's real length, the "did we
+ // run off the end" probe every caller below uses) used to return
+ // CONTAINING_STRUCT(p, ...) on the sentinel itself -- not a real
+ // GAME_MOVE, just g_GameData.sMoveList's own DLIST_ENTRY links
+ // reinterpreted as one, so q->mv/q->szMoveInSan read whatever
+ // bytes happen to follow it in memory. DumpGameList's "dump moves"
+ // command crashed on this immediately (Trace() handed a garbage,
+ // likely-unterminated szMoveInSan pointer); book.c's two callers
+ // have the exact same exposure. Must also confirm p is still a
+ // real node, not the sentinel, before treating it as one.
+ if ((n == 0) && (p != &(g_GameData.sMoveList)))
{
q = CONTAINING_STRUCT(p, GAME_MOVE, links);
return(q);