From 4168718d5d17aba0dea7c816b13c881b656c6260 Mon Sep 17 00:00:00 2001 From: "nethack.rankin" Date: Sat, 10 Feb 2007 04:05:47 +0000 Subject: [PATCH] fix #Q159 - segfault with super long item names From a bug report, putting an object with really long specific and type names into a container with really long specific and type names caused the program to crash. pline() overflowed the buffer it formatted into, and even though it was able to send that for output and trigger a --More-- prompt, eventually a segfault occurred. Give vpline and vraw_printf a much bigger working buffer, then truncate the formatted text to BUFSZ - 1 characters so that we don't just push the long line trouble into the separate interfaces. --- doc/fixes34.4 | 2 ++ src/pline.c | 35 ++++++++++++++++++++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/doc/fixes34.4 b/doc/fixes34.4 index 4d5d019e2..02316e495 100644 --- a/doc/fixes34.4 +++ b/doc/fixes34.4 @@ -313,6 +313,8 @@ surviving choking while eating various foods (cockatrice egg, fortune cookie, wolfsbane, others) didn't carry through to those foods' side-effects shapechangers who take on mimic or hider form will mimic or hide when feasible avoid War message if tinning a Rider corpse fails +prevent long messages from triggering access violation or segmentation fault + due to buffer overflow in pline() Platform- and/or Interface-Specific Fixes diff --git a/src/pline.c b/src/pline.c index eee589505..df8920ae2 100644 --- a/src/pline.c +++ b/src/pline.c @@ -1,4 +1,4 @@ -/* SCCS Id: @(#)pline.c 3.5 2007/01/17 */ +/* SCCS Id: @(#)pline.c 3.5 2007/02/09 */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /* NetHack may be freely redistributed. See license for details. */ @@ -41,7 +41,8 @@ void pline VA_DECL(const char *, line) #endif /* USE_STDARG | USE_VARARG */ - char pbuf[BUFSZ]; + char pbuf[3*BUFSZ]; + int ln; /* Do NOT use VA_START and VA_END in here... see above */ if (!line || !*line) return; @@ -56,6 +57,20 @@ pline VA_DECL(const char *, line) Vsprintf(pbuf,line,VA_ARGS); line = pbuf; } + if ((ln = (int)strlen(line)) > BUFSZ-1) { + if (line != pbuf) /* no '%' was present */ + (void)strncpy(pbuf, line, BUFSZ-1); /* caveat: unterminated */ + /* truncate, preserving the final 3 characters: + "___ extremely long text" -> "___ extremely l...ext" + (this may be suboptimal if overflow is less than 3) */ + (void)strncpy(pbuf + BUFSZ-1 - 6, "...", 3); + /* avoid strncpy; buffers could overlap if excess is small */ + pbuf[BUFSZ-1 - 3] = line[ln - 3]; + pbuf[BUFSZ-1 - 2] = line[ln - 2]; + pbuf[BUFSZ-1 - 1] = line[ln - 1]; + pbuf[BUFSZ-1] = '\0'; + line = pbuf; + } if (!iflags.window_inited) { raw_print(line); return; @@ -263,15 +278,21 @@ vraw_printf(line, the_args) const char *line; va_list the_args; { void raw_printf VA_DECL(const char *, line) #endif + + char pbuf[3*BUFSZ]; + int ln; /* Do NOT use VA_START and VA_END in here... see above */ - if(!index(line, '%')) - raw_print(line); - else { - char pbuf[BUFSZ]; + if (index(line, '%')) { Vsprintf(pbuf,line,VA_ARGS); - raw_print(pbuf); + line = pbuf; } + if ((ln = (int)strlen(line)) > BUFSZ-1) { + if (line != pbuf) line = strncpy(pbuf, line, BUFSZ-1); + /* unlike pline, we don't futz around to keep last few chars */ + pbuf[BUFSZ-1] = '\0'; /* terminate strncpy or truncate vsprintf */ + } + raw_print(line); }