nethack-gwaa-kiwi/nethack
1
0
Fork 0
Code Pull Requests Activity

dynamic format strings vulnerable to user input

Browse Source 
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....
This commit is contained in:
PatR
2017-06-07 11:39:24 -07:00
parent c377b584fc
commit 964fd0fdbd
5 changed files with 66 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
doc/fixes36.1
View File

@@ -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

3
include/extern.h
View File

@@ -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));

52
src/hacklib.c
View File

@@ -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)

8
src/mhitm.c
View File

@@ -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);

8
src/uhitm.c
View File

@@ -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!";