Skip to content

Commit

Permalink
fix exploitable security bug in options processing
Browse files Browse the repository at this point in the history
     From a bug report, the function escapes(),
which is used during options parsing for various options that accept
string values, is given user-controlled input that could end with a
backslash or caret (or two character "\M").  Such a malformed escape
sequence would make it consume the input's end-of-string character and
then keep processing whatever followed.  That meant that it could
generate more data than its output buffer was prepared to hold, making
nethack be vulnerable to stack overflow issues.

     His example that was supposed to clobber the stack didn't trigger
any trouble for me, and I didn't bother trying the second one that can
allegedly cause the Win32 binary to run another program.  But the bug
itself is clearly real.
  • Loading branch information
nethack.rankin committed Aug 3, 2011
1 parent 28ab933 commit 50e12a8
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 22 deletions.
3 changes: 3 additions & 0 deletions doc/fixes34.4
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,9 @@ unlit candelabrum would become unlightable if its candles had exactly 1 turn
temporary loss of Dex from wounded legs will become permanent if it occurs
while mounted and hero dismounts before steed's legs have healed
jaberwocks don't have hands
character escape sequence handling during options processing was vulernable
to malformed escapes and could potentially be abused to clobber the
stack and launch a buffer overrun attack


Platform- and/or Interface-Specific Fixes
Expand Down
44 changes: 22 additions & 22 deletions src/options.c
Original file line number Diff line number Diff line change
Expand Up @@ -747,45 +747,47 @@ nmcpy(dest, src, maxlen)
}

/*
* escapes: escape expansion for showsyms. C-style escapes understood include
* \n, \b, \t, \r, \xnnn (hex), \onnn (octal), \nnn (decimal). The ^-prefix
* for control characters is also understood, and \[mM] followed by any of the
* previous forms or by a character has the effect of 'meta'-ing the value (so
* that the alternate character set will be enabled).
* escapes(): escape expansion for showsyms. C-style escapes understood
* include \n, \b, \t, \r, \xnnn (hex), \onnn (octal), \nnn (decimal).
* The ^-prefix for control characters is also understood, and \[mM]
* has the effect of 'meta'-ing the value which follows (so that the
* alternate character set will be enabled).
*
* For 3.4.3 and earlier, ending with "\M", backslash, or caret prior
* to terminating '\0' would pull that '\0' into the output and then
* keep processing past it. Now, a trailing escape will be handled
* as if it was preceded with its own backslash and be kept as is,
* with end of string terminator always honored as end of input.
*/
STATIC_OVL void
escapes(cp, tp)
const char *cp;
char *tp;
{
while (*cp)
{
while (*cp) {
int cval = 0, meta = 0;

if (*cp == '\\' && index("mM", cp[1])) {
if (*cp == '\\' && cp[1] && index("mM", cp[1]) && cp[2]) {
meta = 1;
cp += 2;
}
if (*cp == '\\' && index("0123456789xXoO", cp[1]))
{
const char *dp, *hex = "00112233445566778899aAbBcCdDeEfF";
if (*cp == '\\' && cp[1] && index("0123456789xXoO", cp[1]) && cp[2]) {
NEARDATA const char hex[] = "00112233445566778899aAbBcCdDeEfF";
const char *dp;
int dcount = 0;

cp++;
if (*cp == 'x' || *cp == 'X')
for (++cp; *cp && (dp = index(hex, *cp)) && (dcount++ < 2); cp++)
cval = (int)((cval * 16) + (dp - hex) / 2);
cval = (cval * 16) + ((int)(dp - hex) / 2);
else if (*cp == 'o' || *cp == 'O')
for (++cp; *cp && (index("01234567",*cp)) && (dcount++ < 3); cp++)
cval = (cval * 8) + (*cp - '0');
else
for (; *cp && (index("0123456789",*cp)) && (dcount++ < 3); cp++)
cval = (cval * 10) + (*cp - '0');
}
else if (*cp == '\\') /* C-style character escapes */
{
switch (*++cp)
{
} else if (*cp == '\\' && cp[1]) { /* C-style character escapes */
switch (*++cp) {
case '\\': cval = '\\'; break;
case 'n': cval = '\n'; break;
case 't': cval = '\t'; break;
Expand All @@ -794,14 +796,12 @@ char *tp;
default: cval = *cp;
}
cp++;
}
else if (*cp == '^') /* expand control-character syntax */
{
} else if (*cp == '^' && cp[1]) { /* expand control-character syntax */
cval = (*++cp & 0x1f);
cp++;
}
else
} else
cval = *cp++;

if (meta)
cval |= 0x80;
*tp++ = cval;
Expand Down

0 comments on commit 50e12a8

Please sign in to comment.