From cdc598e8bdf725eb79265ff9e0e16ae779046b09 Mon Sep 17 00:00:00 2001
From: PatR <rankin@nethack.org>
Date: Tue, 14 Jan 2020 02:52:34 -0800
Subject: [PATCH] fix pline.c potential buffer overruns

Fix 'Bugs 4, 5, and 6' which all use a similar fix but would have
conflicts over '#define BIGBUFSZ' if committed separately.

Format ("short explanation %s", string_argument), where the
explanation always has modest length but the string is potentially
up to 4*BUFSZ in length, into a 5*BUFSZ buffer.  Then truncate the
result to at most BUFSZ-1 characters so that it can be safely passed
to interface-specific putstr() or raw_print().

Applies to pline(), raw_printf(), and config_error_add().  Also done
for impossible() although there's no evidence that its buffer could
be overflowed in a controlled manner.
---
 doc/fixes36.5 |  1 +
 src/pline.c   | 15 +++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/doc/fixes36.5 b/doc/fixes36.5
index a44081a06..9085747ab 100644
--- a/doc/fixes36.5
+++ b/doc/fixes36.5
@@ -10,6 +10,7 @@ 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()
+fix potential buffer overflow in pline(), raw_printf(), and config_error_add()
 
 
 Fixes to Post-3.6.4 Problems that Were Exposed Via git Repository
diff --git a/src/pline.c b/src/pline.c
index b1ac1dffc..9b5fc31de 100644
--- a/src/pline.c
+++ b/src/pline.c
@@ -6,6 +6,10 @@
 #define NEED_VARARGS /* Uses ... */ /* comment line for pre-compiled headers */
 #include "hack.h"
 
+#define BIGBUFSZ (5 * BUFSZ) /* big enough to format a 4*BUFSZ string (from
+                              * config file parsing) with modest decoration;
+                              * result will then be truncated to BUFSZ-1 */
+
 static unsigned pline_flags = 0;
 static char prevmsg[BUFSZ];
 
@@ -118,7 +122,7 @@ VA_DECL(const char *, line)
 #endif /* USE_STDARG | USE_VARARG */
 {       /* start of vpline() or of nested block in USE_OLDARG's pline() */
     static int in_pline = 0;
-    char pbuf[3 * BUFSZ];
+    char pbuf[BIGBUFSZ]; /* will get chopped down to BUFSZ-1 if longer */
     int ln;
     int msgtyp;
     boolean no_repeat;
@@ -443,15 +447,14 @@ void raw_printf
 VA_DECL(const char *, line)
 #endif
 {
-    char pbuf[3 * BUFSZ];
-    int ln;
+    char pbuf[BIGBUFSZ]; /* will be chopped down to BUFSZ-1 if longer */
     /* Do NOT use VA_START and VA_END in here... see above */
 
     if (index(line, '%')) {
         Vsprintf(pbuf, line, VA_ARGS);
         line = pbuf;
     }
-    if ((ln = (int) strlen(line)) > BUFSZ - 1) {
+    if ((int) strlen(line) > BUFSZ - 1) {
         if (line != pbuf)
             line = strncpy(pbuf, line, BUFSZ - 1);
         /* unlike pline, we don't futz around to keep last few chars */
@@ -470,7 +473,7 @@ VA_DECL(const char *, line)
 void impossible
 VA_DECL(const char *, s)
 {
-    char pbuf[2 * BUFSZ];
+    char pbuf[BIGBUFSZ]; /* will be chopped down to BUFSZ-1 if longer */
 
     VA_START(s);
     VA_INIT(s, const char *);
@@ -571,7 +574,7 @@ config_error_add
 VA_DECL(const char *, str)
 #endif /* ?(USE_STDARG || USE_VARARG) */
 {       /* start of vconf...() or of nested block in USE_OLDARG's conf...() */
-    char buf[2 * BUFSZ];
+    char buf[BIGBUFSZ]; /* will be chopped down to BUFSZ-1 if longer */
 
     Vsprintf(buf, str, VA_ARGS);
     buf[BUFSZ - 1] = '\0';