Merge fixes from 'NetHack-3.6' into NetHack-3.7-Jan2020

doc/fixes36.5

@@ -13,6 +13,9 @@ 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()
+	via bad config file values or command line arguments
+fix potential buffer overflow in choose_windows()
+use vsnprintf instead of vsprintf in pline.c where possible
 
 
 Fixes to Post-3.6.4 Problems that Were Exposed Via git Repository

include/patchlevel.h

@@ -46,20 +46,15 @@
 
 /*  Patch 5, January ??, 2020
  *
- * FIXME: update entries depending on whether the cherry-picks are included
- *  -- start cherry-pick entries
  *  fix accessing mons[-1] when trying to gate in a non-valid demon
  *  fix accessing mons[-1] when monster figures out if a tin cures stoning
- *  walking out of tethered-to-buried-object trap condition was supposed to
- *      reinstate punishment but wasn't finding the buried iron ball because
- *      the trap condition was cleared first to indicate escape; result was
- *      attached chain that got dragged around but had no ball attached
- *  -- end cherry-pick entries
  *  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()0
  *  fix potential buffer overflow in sym_val()
  *  fix potential buffer overflow in pline(), raw_printf(), and config_error_add()
+ *  fix potential buffer overflow in choose_windows()
+ *  use vsnprintf instead of vsprintf in pline.c where possible
  *  Windows: incldues a fix from a 3.6.4 post-release update where 
  *      OPTIONS=map_mode:fit_to_screen could cause a game start failure
  */

include/vmsconf.h

@@ -168,6 +168,11 @@ PANICTRACE_GDB=2  #at conclusion of panic, show a call traceback and then
 
 #define FCMASK 0660 /* file creation mask */
 
+/*
+ * 
+ */
+#define NO_VSNPRINTF /* Avoid vsnprintf, use less-safe vsprintf instead. */
+
 /*
  * The remainder of the file should not need to be changed.
  */

src/end.c

@@ -610,7 +610,11 @@ VA_DECL(const char *, str)
     {
         char buf[BUFSZ];
 
+#if !defined(NO_VSNPRINTF)
+        (void) vsnprintf(buf, sizeof buf, str, VA_ARGS);
+#else
         Vsprintf(buf, str, VA_ARGS);
+#endif
         raw_print(buf);
         paniclog("panic", buf);
     }

src/pline.c

@@ -121,6 +121,9 @@ VA_DECL(const char *, line)
     char pbuf[BIGBUFSZ]; /* will get chopped down to BUFSZ-1 if longer */
     int ln;
     int msgtyp;
+#if !defined(NO_VSNPRINTF)
+    int vlen = 0;
+#endif
     boolean no_repeat;
     /* Do NOT use VA_START and VA_END in here... see above */
 
@@ -134,7 +137,16 @@ VA_DECL(const char *, line)
         return;
 
     if (index(line, '%')) {
+#if !defined(NO_VSNPRINTF)
+        vlen = vsnprintf(pbuf, sizeof pbuf, line, VA_ARGS);
+#if (NH_DEVEL_STATUS != NH_STATUS_RELEASED) && defined(DEBUG)
+        if (vlen >= (int) sizeof pbuf)
+            panic("%s: truncation of buffer at %zu of %d bytes",
+                  "pline", sizeof pbuf, vlen);
+#endif
+#else
         Vsprintf(pbuf, line, VA_ARGS);
+#endif
         line = pbuf;
     }
     if ((ln = (int) strlen(line)) > BUFSZ - 1) {
@@ -443,7 +455,11 @@ VA_DECL(const char *, line)
     /* Do NOT use VA_START and VA_END in here... see above */
 
     if (index(line, '%')) {
+#if !defined(NO_VSNPRINTF)
+        (void) vsnprintf(pbuf, sizeof pbuf, line, VA_ARGS);
+#else
         Vsprintf(pbuf, line, VA_ARGS);
+#endif
         line = pbuf;
     }
     if ((int) strlen(line) > BUFSZ - 1) {
@@ -473,7 +489,11 @@ VA_DECL(const char *, s)
         panic("impossible called impossible");
 
     g.program_state.in_impossible = 1;
+#if !defined(NO_VSNPRINTF)
+    (void) vsnprintf(pbuf, sizeof pbuf, s, VA_ARGS);
+#else
     Vsprintf(pbuf, s, VA_ARGS);
+#endif
     pbuf[BUFSZ - 1] = '\0'; /* sanity */
     paniclog("impossible", pbuf);
     if (iflags.debug_fuzzer)
@@ -570,9 +590,21 @@ 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...() */
+#if !defined(NO_VSNPRINTF)
+    int vlen = 0;
+#endif
     char buf[BIGBUFSZ]; /* will be chopped down to BUFSZ-1 if longer */
 
+#if !defined(NO_VSNPRINTF)
+    vlen = vsnprintf(buf, sizeof buf, str, VA_ARGS);
+#if (NH_DEVEL_STATUS != NH_STATUS_RELEASED) && defined(DEBUG)
+    if (vlen >= (int) sizeof buf)
+        panic("%s: truncation of buffer at %zu of %d bytes",
+              "config_error_add", sizeof buf, vlen);
+#endif
+#else
     Vsprintf(buf, str, VA_ARGS);
+#endif
     buf[BUFSZ - 1] = '\0';
     config_erradd(buf);
 

src/topten.c

@@ -997,6 +997,7 @@ int uid;
  * print selected parts of score list.
  * argc >= 2, with argv[0] untrustworthy (directory names, et al.),
  * and argv[1] starting with "-s".
+ * caveat: some shells might allow argv elements to be arbitrarily long.
  */
 void
 prscore(argc, argv)

src/windows.c

@@ -241,7 +241,8 @@ void
 choose_windows(s)
 const char *s;
 {
-    register int i;
+    int i;
+    char *tmps = 0;
 
     for (i = 0; winchoices[i].procs; i++) {
         if ('+' == winchoices[i].procs->name[0])
@@ -267,9 +268,22 @@ const char *s;
         windowprocs.win_wait_synch = def_wait_synch;
 
     if (!winchoices[0].procs) {
-        raw_printf("No window types?");
+        raw_printf("No window types supported?");
         nh_terminate(EXIT_FAILURE);
     }
+    /* 50: arbitrary, no real window_type names are anywhere near that long;
+       used to prevent potential raw_printf() overflow if user supplies a
+       very long string (on the order of 1200 chars) on the command line
+       (config file options can't get that big; they're truncated at 1023) */
+#define WINDOW_TYPE_MAXLEN 50
+    if (strlen(s) >= WINDOW_TYPE_MAXLEN) {
+        tmps = (char *) alloc(WINDOW_TYPE_MAXLEN);
+        (void) strncpy(tmps, s, WINDOW_TYPE_MAXLEN - 1);
+        tmps[WINDOW_TYPE_MAXLEN - 1] = '\0';
+        s = tmps;
+    }
+#undef WINDOW_TYPE_MAXLEN
+
     if (!winchoices[1].procs) {
         config_error_add(
                      "Window type %s not recognized.  The only choice is: %s",
@@ -291,6 +305,8 @@ const char *s;
         config_error_add("Window type %s not recognized.  Choices are:  %s",
                          s, buf);
     }
+    if (tmps)
+        free((genericptr_t) tmps) /*, tmps = 0*/ ;
 
     if (windowprocs.win_raw_print == def_raw_print
             || WINDOWPORT("safe-startup"))

sys/unix/unixmain.c

@@ -355,6 +355,7 @@ char *argv[];
     return 0;
 }
 
+/* caveat: argv elements might be arbitrary long */
 static void
 process_options(argc, argv)
 int argc;
@@ -383,11 +384,10 @@ char *argv[];
                 load_symset("DECGraphics", PRIMARY);
                 switch_symbols(TRUE);
             } else {
-                raw_printf("Unknown option: %s", *argv);
+                raw_printf("Unknown option: %.60s", *argv);
             }
             break;
         case 'X':
-
             discover = TRUE, wizard = FALSE;
             break;
 #ifdef NEWS
@@ -413,7 +413,7 @@ char *argv[];
                 load_symset("RogueIBM", ROGUESET);
                 switch_symbols(TRUE);
             } else {
-                raw_printf("Unknown option: %s", *argv);
+                raw_printf("Unknown option: %.60s", *argv);
             }
             break;
         case 'p': /* profession (role) */
@@ -451,7 +451,7 @@ char *argv[];
                 flags.initrole = i;
                 break;
             }
-            /* else raw_printf("Unknown option: %s", *argv); */
+            /* else raw_printf("Unknown option: %.60s", *argv); */
         }
     }
 

sys/winnt/nttty.c

@@ -1162,7 +1162,7 @@ VA_DECL(const char *, s)
     if (iflags.window_inited)
         end_screen();
     buf[0] = '\n';
-    (void) vsprintf(&buf[1], s, VA_ARGS);
+    (void) vsnprintf(&buf[1], sizeof buf - 1, s, VA_ARGS);
     msmsg(buf);
     really_move_cursor();
     VA_END();
@@ -1980,7 +1980,7 @@ VA_DECL(const char *, fmt)
     char buf[ROWNO * COLNO]; /* worst case scenario */
     VA_START(fmt);
     VA_INIT(fmt, const char *);
-    Vsprintf(buf, fmt, VA_ARGS);
+    (void) vsnprintf(buf, sizeof buf, fmt, VA_ARGS);
     if (redirect_stdout)
         fprintf(stdout, "%s", buf);
     else {

sys/winnt/winnt.c

@@ -241,11 +241,11 @@ VA_DECL(const char *, s)
         end_screen();
     if (WINDOWPORT("tty")) {
         buf[0] = '\n';
-        (void) vsprintf(&buf[1], s, VA_ARGS);
+        (void) vsnprintf(&buf[1], sizeof buf - (1 + sizeof "\n"), s, VA_ARGS);
         Strcat(buf, "\n");
         msmsg(buf);
     } else {
-        (void) vsprintf(buf, s, VA_ARGS);
+        (void) vsnprintf(buf, sizeof buf - sizeof "\n", s, VA_ARGS);
         Strcat(buf, "\n");
         raw_printf(buf);
     }