Repository navigation
Fix Ctrl-R segfault and comma-in-name high-score wipe - #11
Merged
Merged
Conversation
The ^R command in hack.do.c calls pline(0), expecting pline() to
redisplay the previous message. The 1982 pline() handled that case
(print the saved ptr, or "No message." if none), but then fell through
without returning and ran index(line,'%') on the null line pointer,
crashing with SIGSEGV. The #ifndef SMALL guard compiles that branch out
entirely when SMALL is defined, leaving no null handling at all.
Add a return after the existing #ifndef SMALL "No message." branch so it
no longer falls through, and add a matching #else branch for SMALL builds
that prints the same "No message." text instead of dereferencing NULL.
No new behaviour is invented and pline() is not restructured.
Verified in tmux on plain Debug and ASan/UBSan builds: ^R before any
message prints "No message."; after a message ("Unknown command ','")
^R redisplays it; the game keeps running afterwards and ASan stderr is
empty.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The record file stores lines as 'level points name,death' and done() reads them back with fscanf splitting on the first comma. The o command accepted name=x,AAAA... of any length, so the tail of the name landed in the death field, and the unbounded %[^\n] overflowed death[20] (and %[^,] could overflow the 100-byte buf). The next game end then crashed in done() after the record had been reopened for writing, leaving the shared high-score file at 0 bytes. Bound the fscanf fields to their buffers (%99[^,], %19[^\n]) and discard the rest of an over-long line, so a malformed record cannot overflow. Replace ',' with '_' in a name set via name= so new entries never contain the field separator. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two S1 crash fixes from the bug sweep that are live-demo risks. One commit each.
Ctrl-R segfault (P3-02, 1982)
^Rcallspline(0).SMALLis not defined in the real build (hackfoo.hdefines it, but nothing includes it), so the#ifndef SMALLbranch runs. It prints the last message and then falls through toindex(NULL,'%')→ SIGSEGV. The fix adds the missingreturn, plus a guarded "No message." path for SMALL builds.Comma in player name wipes the high-score file (P3-04, 1982)
o→name=x,AAAA…puts a comma, the record's field separator, into the name.done()reads the record back with an unboundedfscanf("%[^,],%[^\n]"), so the next game end overflowsdeath[20]and crashes after the record was reopened for writing. The shared high-score file is left at 0 bytes. The fix bounds the fields (%99[^,],%19[^\n], then discards the rest of the line) and replaces,with_inname=.Verification
Fresh plain and ASan/UBSan builds, tmux playtest:
x_AAAA…. Three games in a row end normally, and all 5 original entries are kept.Known follow-up: a name over 99 characters could still make the parser drop later entries. That's only reachable through the
getlinoverflow, which the stacked S1 PR fixes (P3-05).🤖 Generated with Claude Code