CVE-2020-5210: Buffer Copy without Checking Size of Input ('Classic Buffer Overflow') (CWE-120)
value: In NetHack before 3.6.5, an invalid argument to the -w command line option can cause a buffer overflow resulting in a crash or remote code execution/privilege escalation. This vulnerability affects systems that have NetHack installed suid/sgid and shared systems that allow users to influence command line options. Users should upgrade to NetHack 3.6.5.
CWE description: The product copies an input buffer to an output buffer without verifying that the size of the input buffer is less than the size of the output buffer, leading to a buffer overflow.
CVSS v3 base score: 7.8
Published: 2020-01-28T18:15Z
Repository: https://github.com/NetHack/NetHack
Language: C
Files changed: doc/fixes36.5, src/topten.c, src/windows.c, sys/unix/unixmain.c
Commit date: 2020-01-20 16:08:59 -0500
Commit message: command line triggered buffer overruns

Prevent extremely long command line arguments from overflowing local
buffers in raw_printf or config_error_add.  The increased buffer
sizes they recently got to deal with long configuration file values
aren't sufficient to handle command line induced overflows.

choose_windows(core): copy and truncate the window_type argument in
case it gets passed to config_error_add().

process_options(unix): report bad values with "%.60s" so that vsprintf
will implicitly truncate when formatted by raw_printf().

Diff: diff --git a/doc/fixes36.5 b/doc/fixes36.5
index 54bb65f72..18b38439a 100644
--- a/doc/fixes36.5
+++ b/doc/fixes36.5
@@ -11,10 +11,12 @@ fix accessing mons[-1] when monster figures out if a tin cures stoning
 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()
+	via bad config file values or command line arguments
+fix potential buffer overflow in choose_windows()
 
 
 Fixes to Post-3.6.4 Problems that Were Exposed Via git Repository
 ------------------------------------------------------------------
 
diff --git a/src/topten.c b/src/topten.c
index 6a226df3f..7515c35e3 100644
--- a/src/topten.c
+++ b/src/topten.c
@@ -998,10 +998,11 @@ 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)
 int argc;
 char **argv;
diff --git a/src/windows.c b/src/windows.c
index 5fae44db3..01ef4ccb9 100644
--- a/src/windows.c
+++ b/src/windows.c
@@ -241,11 +241,12 @@ const char *s;
 
 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])
             continue;
         if ('-' == winchoices[i].procs->name[0])
@@ -267,13 +268,26 @@ const char *s;
     if (!windowprocs.win_wait_synch)
         /* early config file error processing routines call this */
         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",
                          s, winchoices[0].procs->name);
     } else {
@@ -291,10 +305,12 @@ const char *s;
             first = FALSE;
         }
         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"))
         nh_terminate(EXIT_SUCCESS);
 }
diff --git a/sys/unix/unixmain.c b/sys/unix/unixmain.c
index c2690ce4a..1e7880165 100644
--- a/sys/unix/unixmain.c
+++ b/sys/unix/unixmain.c
@@ -353,10 +353,11 @@ char *argv[];
     exit(EXIT_SUCCESS);
     /*NOTREACHED*/
     return 0;
 }
 
+/* caveat: argv elements might be arbitrary long */
 static void
 process_options(argc, argv)
 int argc;
 char *argv[];
 {
@@ -381,15 +382,14 @@ char *argv[];
                 wizard = TRUE, discover = FALSE;
             } else if (!strncmpi(*argv, "-DECgraphics", l)) {
                 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
         case 'n':
             iflags.news = FALSE;
@@ -411,11 +411,11 @@ char *argv[];
             if (!strncmpi(*argv, "-IBMgraphics", l)) {
                 load_symset("IBMGraphics", PRIMARY);
                 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) */
             if (argv[0][2]) {
                 if ((i = str2role(&argv[0][2])) >= 0)
@@ -449,11 +449,11 @@ char *argv[];
         default:
             if ((i = str2role(&argv[0][1])) >= 0) {
                 flags.initrole = i;
                 break;
             }
-            /* else raw_printf("Unknown option: %s", *argv); */
+            /* else raw_printf("Unknown option: %.60s", *argv); */
         }
     }
 
 #ifdef SYSCF
     if (argc > 1)

Vulnerable code:     register int i;
        raw_printf("No window types?");
                raw_printf("Unknown option: %s", *argv);

                raw_printf("Unknown option: %s", *argv);
            /* else raw_printf("Unknown option: %s", *argv); */
Fixed code: 	via bad config file values or command line arguments
fix potential buffer overflow in choose_windows()
 * caveat: some shells might allow argv elements to be arbitrarily long.
    int i;
    char *tmps = 0;
        raw_printf("No window types supported?");
    /* 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 (tmps)
        free((genericptr_t) tmps) /*, tmps = 0*/ ;
/* caveat: argv elements might be arbitrary long */
                raw_printf("Unknown option: %.60s", *argv);
                raw_printf("Unknown option: %.60s", *argv);
            /* else raw_printf("Unknown option: %.60s", *argv); */