Skip to content

Commit 4f81bda

Browse files
committed
Fix: buffer overflow in gamelog with an extremely long wish string
This was discovered when a game of xNetHack crashed with stack smashing detected during dumplog creation after an ascension. I traced the problem to a wish with a very long string the player had made much earlier in the game ("greased very blessed holy rustproof unlit historic thoroughly +5 very cloak of protection named it would be a shame if something happened to me wearing this cloak"), which is further recorded in an even longer form in the chronicle as 'wished for "X", got "Y"'. That string does get truncated, but since the gamelog strings are dynamically allocated, they can be longer than BUFSZ. When show_gamelog was subsequently called, it didn't use any bounds checking, which allowed its stack-allocated buffer to overflow. Changing the offending sprintf to snprintf and limiting it to the buffer size appears to fix this issue. It will truncate the string at BUFSZ-1 characters and therefore will be expressed in the dumplog as an incomplete string, but 1) that was happening anyway because the gamelog string already doesn't capture the entire "wished for X, got Y" message on such a wish, and 2) this should only ever happen for very long wishes.
1 parent f6a544a commit 4f81bda

1 file changed

Lines changed: 1 addition & 1 deletion

File tree

src/insight.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2695,7 +2695,7 @@ show_gamelog(int final)
26952695
continue;
26962696
if (!eventcnt++)
26972697
putstr(win, ATR_SUBHEAD, " Turn");
2698-
Sprintf(buf, "%5ld: %s", llmsg->turn, llmsg->text);
2698+
Snprintf(buf, sizeof buf, "%5ld: %s", llmsg->turn, llmsg->text);
26992699
putstr(win, 0, buf);
27002700
}
27012701
/* since start of game is logged as a major event, 'eventcnt' should

0 commit comments

Comments
 (0)