From 50e12a87aa1b24d382c03d524c71168027f3bcd6 Mon Sep 17 00:00:00 2001
From: "nethack.rankin" <nethack.rankin>
Date: Wed, 3 Aug 2011 12:42:12 +0000
Subject: [PATCH] fix exploitable security bug in options processing

     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.
---
 doc/fixes34.4 |  3 +++
 src/options.c | 44 ++++++++++++++++++++++----------------------
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/doc/fixes34.4 b/doc/fixes34.4
index eea2e2d95..b259db9e3 100644
--- a/doc/fixes34.4
+++ b/doc/fixes34.4
@@ -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
diff --git a/src/options.c b/src/options.c
index e36f91ca7..c4a696aa0 100644
--- a/src/options.c
+++ b/src/options.c
@@ -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;
@@ -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;
-- 
2.50.1