From: PatR Date: Wed, 7 Jun 2017 18:39:24 +0000 (-0700) Subject: dynamic format strings vulnerable to user input X-Git-Tag: NetHack-3.6.1_RC01~479 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=964fd0fdbd16d22421a2591d8c907a8f5cc1214a;p=nethack dynamic format strings vulnerable to user input This adds new utility routine strNsubst(), a more versatile version of the existing strsubst(), that can replace the Nth occurrence of a substring rather than just the first, and replaces all occurrences if N is 0. When working on vampire shape-shifting messages a few days ago I noticed that a constructed pline/sprintf format was vulnerable to the player giving the vampire a name with '%' in it and included a fix for that. This fixes two other instances of the same vulnerability: a monster with reflection triggering a floating eye's gaze and the hero using a silver weapon against a silver- hating monster. I didn't do a lot of experimenting with the failure, just assigned the name "foo%s" to the floating eye or the weapon. The resulting feedback for the relevant messages was garbled due to parameters being substituted in the wrong place. When that caused there to be too few arguments to satisfy the format, the final message included "null" for the missing one rather than triggering a crash while trying to format something arbitrary from the stack. I don't think these bugs provided sufficient user control to be vulnerable to stack manipulation that does something naughty. I found the dynamic format strings by searching for "%%". There may be others scattered around the code which don't have that as an indicator.... --- diff --git a/doc/fixes36.1 b/doc/fixes36.1 index d010c2541..4a43125d1 100644 --- a/doc/fixes36.1 +++ b/doc/fixes36.1 @@ -393,6 +393,9 @@ poor message when named vampire shifts shape within view: You observe a Dracula where a Dracula was. vampire shifting into fog cloud to pass under door "oozed" rather than "flowed" adult green dragons and the Chromatic Dragon were blinded by gas clouds +named floating eye (when hit by another monster with reflection) or named + silver weapon (when hero hits silver-hating monster) could disrupt + message formatting and conceivably trigger crash if name had '%' in it Fixes to Post-3.6.0 Problems that Were Exposed Via git Repository diff --git a/include/extern.h b/include/extern.h index 98a258cef..58a451c67 100644 --- a/include/extern.h +++ b/include/extern.h @@ -1,4 +1,4 @@ -/* NetHack 3.6 extern.h $NHDT-Date: 1496531111 2017/06/03 23:05:11 $ $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.589 $ */ +/* NetHack 3.6 extern.h $NHDT-Date: 1496860756 2017/06/07 18:39:16 $ $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.590 $ */ /* Copyright (c) Steve Creps, 1988. */ /* NetHack may be freely redistributed. See license for details. */ @@ -876,6 +876,7 @@ E boolean FDECL(onlyspace, (const char *)); E char *FDECL(tabexpand, (char *)); E char *FDECL(visctrl, (CHAR_P)); E char *FDECL(strsubst, (char *, const char *, const char *)); +E int FDECL(strNsubst, (char *, const char *, const char *, int)); E const char *FDECL(ordin, (int)); E char *FDECL(sitoa, (int)); E int FDECL(sgn, (int)); diff --git a/src/hacklib.c b/src/hacklib.c index 8cb76021b..1c4fe63d6 100644 --- a/src/hacklib.c +++ b/src/hacklib.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 hacklib.c $NHDT-Date: 1472006251 2016/08/24 02:37:31 $ $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.48 $ */ +/* NetHack 3.6 hacklib.c $NHDT-Date: 1496860756 2017/06/07 18:39:16 $ $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.50 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /* Copyright (c) Robert Patrick Rankin, 1991 */ /* NetHack may be freely redistributed. See license for details. */ @@ -33,6 +33,7 @@ char * tabexpand (char *) char * visctrl (char) char * strsubst (char *, const char *, const char *) + int strNsubst (char *,const char *,const char *,int) const char * ordin (int) char * sitoa (int) int sgn (int) @@ -45,7 +46,7 @@ boolean pmatchz (const char *, const char *) int strncmpi (const char *, const char *, int) char * strstri (const char *, const char *) - boolean fuzzymatch (const char *,const char *, + boolean fuzzymatch (const char *, const char *, const char *, boolean) void setrandom (void) time_t getnow (void) @@ -442,6 +443,7 @@ const char *orig, *replacement; char *found, buf[BUFSZ]; if (bp) { + /* [this could be replaced by strNsubst(bp, orig, replacement, 1)] */ found = strstr(bp, orig); if (found) { Strcpy(buf, found + strlen(orig)); @@ -452,6 +454,52 @@ const char *orig, *replacement; return bp; } +/* substitute the Nth occurrence of a substring within a string (in place); + if N is 0, substitute all occurrences; returns the number of subsitutions; + maximum output length is BUFSZ (BUFSZ-1 chars + terminating '\0') */ +int +strNsubst(inoutbuf, orig, replacement, n) +char *inoutbuf; /* current string, and result buffer */ +const char *orig, /* old substring; if "" then insert in front of Nth char */ + *replacement; /* new substring; if "" then delete old substring */ +int n; /* which occurrence to replace; 0 => all */ +{ + char *bp, *op, workbuf[BUFSZ]; + const char *rp; + unsigned len = (unsigned) strlen(orig); + int ocount = 0, /* number of times 'orig' has been matched */ + rcount = 0; /* number of subsitutions made */ + + for (bp = inoutbuf, op = workbuf; *bp && op < &workbuf[BUFSZ - 1]; ) { + if ((!len || !strncmp(bp, orig, len)) && (++ocount == n || n == 0)) { + /* Nth match found */ + for (rp = replacement; *rp && op < &workbuf[BUFSZ - 1]; ) + *op++ = *rp++; + ++rcount; + if (len) { + bp += len; /* skip 'orig' */ + continue; + } + } + /* no match (or len==0) so retain current character */ + *op++ = *bp++; + } + if (!len && n == ocount + 1) { + /* special case: orig=="" (!len) and n==strlen(inoutbuf)+1, + insert in front of terminator (in other words, append); + [when orig=="", ocount will have been incremented once for + each input char] */ + for (rp = replacement; *rp && op < &workbuf[BUFSZ - 1]; ) + *op++ = *rp++; + ++rcount; + } + if (rcount) { + *op = '\0'; + Strcpy(inoutbuf, workbuf); + } + return rcount; +} + /* return the ordinal suffix of a number */ const char * ordin(n) diff --git a/src/mhitm.c b/src/mhitm.c index 38a56404b..00ac76cb1 100644 --- a/src/mhitm.c +++ b/src/mhitm.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 mhitm.c $NHDT-Date: 1496619132 2017/06/04 23:32:12 $ $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.96 $ */ +/* NetHack 3.6 mhitm.c $NHDT-Date: 1496860757 2017/06/07 18:39:17 $ $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.97 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /* NetHack may be freely redistributed. See license for details. */ @@ -1558,8 +1558,10 @@ int mdead; tmp = 127; if (magr->mcansee && haseyes(madat) && mdef->mcansee && (perceives(madat) || !mdef->minvis)) { - Sprintf(buf, "%s gaze is reflected by %%s %%s.", - s_suffix(Monnam(mdef))); + /* construct format string; guard against '%' in Monnam */ + Strcpy(buf, s_suffix(Monnam(mdef))); + (void) strNsubst(buf, "%", "%%", 0); + Strcat(buf, " gaze is reflected by %s %s."); if (mon_reflects(magr, canseemon(magr) ? buf : (char *) 0)) return (mdead | mhit); diff --git a/src/uhitm.c b/src/uhitm.c index a4a31cde0..b11cb0fd8 100644 --- a/src/uhitm.c +++ b/src/uhitm.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 uhitm.c $NHDT-Date: 1470819843 2016/08/10 09:04:03 $ $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.164 $ */ +/* NetHack 3.6 uhitm.c $NHDT-Date: 1496860757 2017/06/07 18:39:17 $ $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.166 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /* NetHack may be freely redistributed. See license for details. */ @@ -1123,9 +1123,13 @@ int thrown; /* HMON_xxx (0 => hand-to-hand, other => ranged) */ else if (barehand_silver_rings == 2) fmt = "Your silver rings sear %s!"; else if (silverobj && saved_oname[0]) { - Sprintf(silverobjbuf, "Your %s%s %s %%s!", + /* guard constructed format string against '%' in + saved_oname[] from xname(via cxname()) */ + Sprintf(silverobjbuf, "Your %s%s %s", strstri(saved_oname, "silver") ? "" : "silver ", saved_oname, vtense(saved_oname, "sear")); + (void) strNsubst(silverobjbuf, "%", "%%", 0); + Strcat(silverobjbuf, " %s!"); fmt = silverobjbuf; } else fmt = "The silver sears %s!";