From 40789b7070128d2f31536c0be422bd3032a5fffc Mon Sep 17 00:00:00 2001 From: PatR Date: Fri, 13 Dec 2019 13:36:38 -0800 Subject: [PATCH] fix potential buffer overflow loading config file --- doc/fixes36.4 | 1 + src/files.c | 35 +++++++++++++++++++++++------------ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/doc/fixes36.4 b/doc/fixes36.4 index 5865224cc..aada6c85f 100644 --- a/doc/fixes36.4 +++ b/doc/fixes36.4 @@ -11,6 +11,7 @@ GDBPATH and GREPPATH from sysconf or -D... on compilation command line were at end of game when that was enabled fix the article used in the message when your steed encounters a polymorph trap dozen-ish assorted spelling/typo fixes in messages and source comments +fix potential buffer overflow when parsing run-time configuration file Fixes to Post-3.6.3 Problems that Were Exposed Via git Repository diff --git a/src/files.c b/src/files.c index 5f24956b8..e598ced9a 100644 --- a/src/files.c +++ b/src/files.c @@ -2309,10 +2309,14 @@ char *origbuf; 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); @@ -3028,7 +3032,11 @@ 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) @@ -3136,22 +3144,23 @@ boolean FDECL((*proc), (char *)); /* 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; @@ -3173,11 +3182,11 @@ boolean FDECL((*proc), (char *)); } 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; } @@ -3294,6 +3303,8 @@ int which_set; 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);