From b32ce258e37affe4dcfe548a69476008ca7a58e7 Mon Sep 17 00:00:00 2001 From: copperwater Date: Sat, 29 Mar 2025 07:11:49 -0400 Subject: [PATCH] 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. --- src/insight.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/insight.c b/src/insight.c index f4d0c945f..a1c44b6c9 100644 --- a/src/insight.c +++ b/src/insight.c @@ -2580,7 +2580,7 @@ show_gamelog(int final) continue; if (!eventcnt++) putstr(win, 0, " Turn"); - Sprintf(buf, "%5ld: %s", llmsg->turn, llmsg->text); + Snprintf(buf, sizeof buf, "%5ld: %s", llmsg->turn, llmsg->text); putstr(win, 0, buf); } /* since start of game is logged as a major event, 'eventcnt' should