From 74de7d31e0a6b3e2ebd852e333fe66d212fd6a90 Mon Sep 17 00:00:00 2001 From: PatR Date: Tue, 14 Jan 2020 02:05:14 -0800 Subject: [PATCH] fix sym_val() buffer overrun Fix 'Bug 3' where too long SYMBOL=string in run-time config file could overflow a local buffer and clobber the stack. Valid value is only one character long after processing an 'escaped' encoded character which can be at most 6 characters (plus terminator): backslash M backslash and up three digits. If/when UTF8 gets added the number of digits will increase. Use a truncated copy of the input (substantially bigger than 6+1); ignore any excess. --- doc/fixes36.5 | 3 ++- src/options.c | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/doc/fixes36.5 b/doc/fixes36.5 index c3cad8fe0..a44081a06 100644 --- a/doc/fixes36.5 +++ b/doc/fixes36.5 @@ -1,4 +1,4 @@ -$NHDT-Branch: NetHack-3.6 $:$NHDT-Revision: 1.2 $ $NHDT-Date: 1578972411 2020/01/14 03:26:51 $ +$NHDT-Branch: NetHack-3.6 $:$NHDT-Revision: 1.3 $ $NHDT-Date: 1578996303 2020/01/14 10:05:03 $ fixes36.5 contains a terse summary of changes made to 3.6.4 in order to produce 3.6.5 as well as any post-release fixes in binaries. @@ -9,6 +9,7 @@ General Fixes and Modified Features have string_for_opt() return empty_optstr on failure ensure existing callers of string_for_opt() check return value before using it fix potential buffer overflow in add_menu_coloring() +fix potential buffer overflow in sym_val() Fixes to Post-3.6.4 Problems that Were Exposed Via git Repository diff --git a/src/options.c b/src/options.c index 3e2582a4e..1f8477193 100644 --- a/src/options.c +++ b/src/options.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 options.c $NHDT-Date: 1578972408 2020/01/14 03:26:48 $ $NHDT-Branch: NetHack-3.6 $:$NHDT-Revision: 1.395 $ */ +/* NetHack 3.6 options.c $NHDT-Date: 1578996303 2020/01/14 10:05:03 $ $NHDT-Branch: NetHack-3.6 $:$NHDT-Revision: 1.396 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Michael Allison, 2008. */ /* NetHack may be freely redistributed. See license for details. */ @@ -952,8 +952,8 @@ int maxlen; */ STATIC_OVL void escapes(cp, tp) -const char *cp; -char *tp; +const char *cp; /* might be 'tp', updating in place */ +char *tp; /* result is never longer than 'cp' */ { static NEARDATA const char oct[] = "01234567", dec[] = "0123456789", hex[] = "00112233445566778899aAbBcCdDeEfF"; @@ -6177,9 +6177,9 @@ char *buf; int sym_val(strval) -const char *strval; +const char *strval; /* up to 4*BUFSZ-1 long; only first few chars matter */ { - char buf[QBUFSZ]; + char buf[QBUFSZ], tmp[QBUFSZ]; /* to hold trucated copy of 'strval' */ buf[0] = '\0'; if (!strval[0] || !strval[1]) { /* empty, or single character */ @@ -6200,7 +6200,7 @@ const char *strval; /* not simple quote or basic backslash; strip closing quote and let escapes() deal with it */ } else { - char *p, tmp[QBUFSZ]; + char *p; (void) strncpy(tmp, strval + 1, sizeof tmp - 1); tmp[sizeof tmp - 1] = '\0'; @@ -6209,8 +6209,11 @@ const char *strval; escapes(tmp, buf); } /* else buf[0] stays '\0' */ } - } else /* not lone char nor single quote */ - escapes(strval, buf); + } else { /* not lone char nor single quote */ + (void) strncpy(tmp, strval + 1, sizeof tmp - 1); + tmp[sizeof tmp - 1] = '\0'; + escapes(tmp, buf); + } return (int) *buf; }