CVE-2019-19905: Buffer Copy without Checking Size of Input ('Classic Buffer Overflow') (CWE-120)
value: NetHack 3.6.x before 3.6.4 is prone to a buffer overflow vulnerability when reading very long lines from configuration files. This affects systems that have NetHack installed suid/sgid, and shared systems that allow users to upload their own configuration files.
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: 9.8
Published: 2019-12-19T18:15Z
Repository: https://github.com/NetHack/NetHack
Language: Other
Files changed: doc/fixes36.4, src/files.c
Commit date: 2019-12-17 14:17:55 -0500
Commit message: fix potential buffer overflow loading config file

Diff: diff --git a/doc/fixes36.4 b/doc/fixes36.4
index f1ec65af..441dd777 100644
--- a/doc/fixes36.4
+++ b/doc/fixes36.4
@@ -1,10 +1,10 @@
 $NHDT-Branch: NetHack-3.6 $:$NHDT-Revision: 1.4 $ $NHDT-Date: 1576287569 2019/12/14 01:39:29 $
 
-This fixes36.4 file is here to capture information about updates in the 3.6.x
-lineage following the release of 3.6.3 in December 2019.  Hypothetical version
-3.6.4 may not be released, in which case these fixes will appear in 3.7.0.
+fixes36.4 contains a terse summary of changes made to 3.6.3 in order to
+produce 3.6.4.
+ 
 
 General Fixes and Modified Features
 -----------------------------------
 GDBPATH and GREPPATH from sysconf or -D... on compilation command line were
 	being processed even if PANICTRACE was disabled but only being freed
@@ -13,30 +13,19 @@ fix the article used in the message when your steed encounters a polymorph trap
 allow teleporting onto the vibrating square
 message "your knapsack can't accomodate any more items" when picking stuff up
 	or removing such from container was inaccurate if there was some gold
 	pending; vary the message rather than add more convoluted pickup code
 dozen-ish assorted spelling/typo fixes in messages and source comments
-flying hero could not use a hole deliberately with '>'
-
-
-Fixes to Post-3.6.3 Problems that Were Exposed Via git Repository
-------------------------------------------------------------------
+fix potential buffer overflow when parsing run-time configuration file
 
 
 Platform- and/or Interface-Specific Fixes or Features
 -----------------------------------------------------
 fix compilation on platforms that split the ncurses and tinfo libraries
 allow run-from-removable-device on Windows
 
 
 General New Features
 --------------------
-
-
-NetHack Community Patches (or Variation) Included
--------------------------------------------------
-
-
-Code Cleanup and Reorganization
--------------------------------
+none
 
 
diff --git a/src/files.c b/src/files.c
index 7f9cf65b..5e500ae3 100644
--- a/src/files.c
+++ b/src/files.c
@@ -2307,14 +2307,18 @@ char *origbuf;
     char *bufp, buf[4 * BUFSZ];
     uchar translate[MAXPCHARS];
     int len;
     boolean retval = TRUE;
 
+    while (*origbuf == ' ' || *origbuf == '\t') /* skip leading whitespace */
+        ++origbuf;                   /* (caller probably already did this) */
+    (void) strncpy(buf, origbuf, sizeof buf - 1);
+    buf[sizeof buf - 1] = '\0'; /* strncpy not guaranteed to NUL terminate */
     /* convert any tab to space, condense consecutive spaces into one,
        remove leading and trailing spaces (exception: if there is nothing
        but spaces, one of them will be kept even though it leads/trails) */
-    mungspaces(strcpy(buf, origbuf));
+    mungspaces(buf);
 
     /* find the '=' or ':' */
     bufp = find_optparam(buf);
     if (!bufp) {
         config_error_add("Not a config statement, missing '='");
@@ -3032,11 +3036,15 @@ struct obj *obj;
 
 boolean
 proc_wizkit_line(buf)
 char *buf;
 {
-    struct obj *otmp = readobjnam(buf, (struct obj *) 0);
+    struct obj *otmp;
+
+    if (strlen(buf) >= BUFSZ)
+        buf[BUFSZ - 1] = '\0';
+    otmp = readobjnam(buf, (struct obj *) 0);
 
     if (otmp) {
         if (otmp != &zeroobj)
             wizkit_addinv(otmp);
     } else {
@@ -3140,26 +3148,27 @@ boolean FDECL((*proc), (char *));
                 if (buf)
                     oldline = TRUE;
 
                 /* merge now read line with previous ones, if necessary */
                 if (!ignoreline) {
-                    len = (int) strlen(inbuf) + 1;
+                    len = (int) strlen(ep) + 1; /* +1: final '\0' */
                     if (buf)
-                        len += (int) strlen(buf);
+                        len += (int) strlen(buf) + 1; /* +1: space */
                     tmpbuf = (char *) alloc(len);
+                    *tmpbuf = '\0';
                     if (buf) {
-                        Sprintf(tmpbuf, "%s %s", buf, inbuf);
+                        Strcat(strcpy(tmpbuf, buf), " ");
                         free(buf);
-                    } else
-                        Strcpy(tmpbuf, inbuf);
-                    buf = tmpbuf;
+                    }
+                    buf = strcat(tmpbuf, ep);
+                    buf[sizeof inbuf - 1] = '\0';
                 }
 
                 if (morelines || (ignoreline && !oldline))
                     continue;
 
-                if (handle_config_section(ep)) {
+                if (handle_config_section(buf)) {
                     free(buf);
                     buf = (char *) 0;
                     continue;
                 }
 
@@ -3177,15 +3186,15 @@ boolean FDECL((*proc), (char *));
                         buf = (char *) 0;
                         continue;
                     }
                     bufp++;
                     if (config_section_chosen)
-                        free(config_section_chosen);
+                        free(config_section_chosen), config_section_chosen = 0;
                     section = choose_random_part(bufp, ',');
-                    if (section)
+                    if (section) {
                         config_section_chosen = dupstr(section);
-                    else {
+                    } else {
                         config_error_add("No config section to choose");
                         rv = FALSE;
                     }
                     free(buf);
                     buf = (char *) 0;
@@ -3298,10 +3307,12 @@ int which_set;
 {
     int val, i;
     struct symparse *symp;
     char *bufp, *commentp, *altp;
 
+    if (strlen(buf) >= BUFSZ)
+        buf[BUFSZ - 1] = '\0';
     /* convert each instance of whitespace (tabs, consecutive spaces)
        into a single space; leading and trailing spaces are stripped */
     mungspaces(buf);
 
     /* remove trailing comment, if any (this isn't strictly needed for

Vulnerable code: This fixes36.4 file is here to capture information about updates in the 3.6.x
lineage following the release of 3.6.3 in December 2019.  Hypothetical version
3.6.4 may not be released, in which case these fixes will appear in 3.7.0.
flying hero could not use a hole deliberately with '>'


Fixes to Post-3.6.3 Problems that Were Exposed Via git Repository


NetHack Community Patches (or Variation) Included


Code Cleanup and Reorganization
    mungspaces(strcpy(buf, origbuf));
    struct obj *otmp = readobjnam(buf, (struct obj *) 0);
                    len = (int) strlen(inbuf) + 1;
                        len += (int) strlen(buf);
                        Sprintf(tmpbuf, "%s %s", buf, inbuf);
                    } else
                        Strcpy(tmpbuf, inbuf);
                    buf = tmpbuf;
                if (handle_config_section(ep)) {
                        free(config_section_chosen);
                    if (section)
                    else {
Fixed code: fixes36.4 contains a terse summary of changes made to 3.6.3 in order to
produce 3.6.4.
 
fix potential buffer overflow when parsing run-time configuration file
none
    while (*origbuf == ' ' || *origbuf == '\t') /* skip leading whitespace */
        ++origbuf;                   /* (caller probably already did this) */
    (void) strncpy(buf, origbuf, sizeof buf - 1);
    buf[sizeof buf - 1] = '\0'; /* strncpy not guaranteed to NUL terminate */
    mungspaces(buf);
    struct obj *otmp;

    if (strlen(buf) >= BUFSZ)
        buf[BUFSZ - 1] = '\0';
    otmp = readobjnam(buf, (struct obj *) 0);
                    len = (int) strlen(ep) + 1; /* +1: final '\0' */
                        len += (int) strlen(buf) + 1; /* +1: space */
                    *tmpbuf = '\0';
                        Strcat(strcpy(tmpbuf, buf), " ");
                    }
                    buf = strcat(tmpbuf, ep);
                    buf[sizeof inbuf - 1] = '\0';
                if (handle_config_section(buf)) {
                        free(config_section_chosen), config_section_chosen = 0;
                    if (section) {
                    } else {
    if (strlen(buf) >= BUFSZ)
        buf[BUFSZ - 1] = '\0';