Skip to content

Fix out-of-bounds read of exitreversaltable in exit1() - #34

Closed
ApolloniaUK wants to merge 1 commit into
DavidKinder:masterfrom
ApolloniaUK:master
Closed

Fix out-of-bounds read of exitreversaltable in exit1()#34
ApolloniaUK wants to merge 1 commit into
DavidKinder:masterfrom
ApolloniaUK:master

Conversation

@ApolloniaUK

@ApolloniaUK ApolloniaUK commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Hi David,

I’ve recently got back to working on my Mac port with a view to incorporating version 5.2 changes and seriously modernizing the code - basically a complete rewrite moving from old Mac Toolbox to AppKit or SwiftUi.

As part of my preparation I’ve been using Claude Code (Fable 5) to give Andreas’s test harness a very thorough workout so I could be sure my test games were all exactly matching Andreas’s. In the process Fable found a tiny bug in the interpreter with the test version of The Price of Magik. Here's Fable's report:

The Exit opcode reads two game variables, one of which is a direction code, and when the forward search finds no matching exit, exit1() looks up the reverse direction with d6 = exitreversaltable[d6] (level9.c:2935). That table is 20 bytes. The v5.2 changelog notes fixing "an array overrun when handling the commands forwards, backwards, left or right" — that fix extended the table with 0xff sentinel entries for direction codes 16–19. But Price of Magik (during the test script, right after CAST FIN turns you into a fish and a WAIT command) passes a direction code of 20 or more, which still indexes past the end of the table.

In C this reads whatever byte happens to sit after the table in the data segment — undefined behaviour that's deterministic for a given build and, by luck, harmless: the garbage value doesn't match any exit nibble, so the game just reports you can't go that way. Harmless at the moment but a compiler upgrade or layout change could someday turn into a real misbehavior.

I had it patch level9.c and I’ve merged that fix from my dev branch back into master. Here it is in this pull request if you are interested. I haven’t bumped version number or changed copyright date - didn’t know whether you would think either necessary for such a tiny tweak.

HTH.

Alan.
(Alan Staniforth)

Direction codes beyond 19 (The Price of Magik passes one) indexed past
the 20-byte table. Treat such codes as unmapped (0xff), extending the
v5.2 fix that covered codes 16-19.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ApolloniaUK
ApolloniaUK marked this pull request as draft July 27, 2026 17:35
@ApolloniaUK
ApolloniaUK marked this pull request as ready for review July 27, 2026 17:39
@ApolloniaUK ApolloniaUK changed the title FIX: Fix out-of-bounds read of exitreversaltable in exit1() Fix out-of-bounds read of exitreversaltable in exit1() Jul 27, 2026
@ApolloniaUK
ApolloniaUK marked this pull request as draft July 28, 2026 08:48
@ApolloniaUK
ApolloniaUK marked this pull request as ready for review July 28, 2026 08:50
@DavidKinder

Copy link
Copy Markdown
Owner

Hi Alan, thanks for this. I wasn't entirely happy with the commit message Claude created, and I'm not entirely thrilled at having Claude as a contributor to any of my personal repos, so I changed the comment and committed it as 67fc925.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants