Skip to content
Permalink
Browse files Browse the repository at this point in the history
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().
  • Loading branch information
PatR authored and nhmall committed Jan 20, 2020
1 parent a8208b4 commit f3def5c
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 6 deletions.
2 changes: 2 additions & 0 deletions doc/fixes36.5
Expand Up @@ -13,6 +13,8 @@ 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
Expand Down
1 change: 1 addition & 0 deletions src/topten.c
Expand Up @@ -1000,6 +1000,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)
Expand Down
20 changes: 18 additions & 2 deletions src/windows.c
Expand Up @@ -243,7 +243,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])
Expand All @@ -269,9 +270,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",
Expand All @@ -293,6 +307,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"))
Expand Down
8 changes: 4 additions & 4 deletions sys/unix/unixmain.c
Expand Up @@ -355,6 +355,7 @@ char *argv[];
return 0;
}

/* caveat: argv elements might be arbitrary long */
static void
process_options(argc, argv)
int argc;
Expand Down Expand Up @@ -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
Expand All @@ -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) */
Expand Down Expand Up @@ -451,7 +451,7 @@ char *argv[];
flags.initrole = i;
break;
}
/* else raw_printf("Unknown option: %s", *argv); */
/* else raw_printf("Unknown option: %.60s", *argv); */
}
}

Expand Down

0 comments on commit f3def5c

Please sign in to comment.