From 730b67b838bc2b1742010b3b9d09081e743e38ed Mon Sep 17 00:00:00 2001 From: PatR Date: Sat, 4 Jan 2020 03:33:57 -0800 Subject: [PATCH] add some bullet-proofing to tabexpand() Include some bounds checking for tabexpand, but it assumes caller passes a BUFSZ buffer rather than having that caller pass the actual size. --- doc/fixes37.0 | 6 +++++- src/hacklib.c | 27 ++++++++++++++++++++------- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/doc/fixes37.0 b/doc/fixes37.0 index 094ab17ae..86dcd6e5a 100644 --- a/doc/fixes37.0 +++ b/doc/fixes37.0 @@ -1,4 +1,4 @@ -$NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.40 $ $NHDT-Date: 1577674533 2019/12/30 02:55:33 $ +$NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.43 $ $NHDT-Date: 1578137629 2020/01/04 11:33:49 $ General Fixes and Modified Features ----------------------------------- @@ -89,3 +89,7 @@ more current Qt for Qt version 4 and 5 moved from win/Qt4 to win/Qt; qt4 moniker changed to qt_ window-port-interface: add_menu() modified to take a more general itemflags parameter to support uses beyond just 'preselected' +add some bounds checking to tabexpand (doesn't prevent the apparent compiler + optimization bug that put homebrew OSX executable into endless loop) + + diff --git a/src/hacklib.c b/src/hacklib.c index d823351f9..7afebd219 100644 --- a/src/hacklib.c +++ b/src/hacklib.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 hacklib.c $NHDT-Date: 1574636502 2019/11/24 23:01:42 $ $NHDT-Branch: paxed-quest-lua $:$NHDT-Revision: 1.79 $ */ +/* NetHack 3.6 hacklib.c $NHDT-Date: 1578137629 2020/01/04 11:33:49 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.80 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Michael Allison, 2007. */ /* Copyright (c) Robert Patrick Rankin, 1991 */ @@ -416,27 +416,40 @@ const char *s; return TRUE; } -/* expand tabs into proper number of spaces */ +/* expand tabs into proper number of spaces (in place) */ char * tabexpand(sbuf) -char *sbuf; +char *sbuf; /* assumed to be [BUFSZ] but can be smaller provided that expanded + * string fits; expansion bigger than BUFSZ-1 will be truncated */ { - char buf[BUFSZ]; + char buf[BUFSZ + 10]; register char *bp, *s = sbuf; register int idx; if (!*s) return sbuf; - /* warning: no bounds checking performed */ - for (bp = buf, idx = 0; *s; s++) + for (bp = buf, idx = 0; *s; s++) { if (*s == '\t') { + /* + * clang-8's optimizer at -O4 has been observed to mis-compile + * this code when unrolling the loop. Symptom is nethack + * getting stuck in an apparent infinite loop (or perhaps just + * an extremely long one) when examining data.base entries. + * clang-9 doesn't exhibit this problem. [Was the incorrect + * optimization fixed or just disabled?] + */ do *bp++ = ' '; while (++idx % 8); } else { *bp++ = *s; - idx++; + ++idx; } + if (idx >= BUFSZ) { + bp = &buf[BUFSZ - 1]; + break; + } + } *bp = 0; return strcpy(sbuf, buf); }