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().
This commit is contained in:
PatR
2020-01-20 16:08:59 -05:00
committed by nhmall
parent a8208b44c8
commit f3def5c0b9
4 changed files with 25 additions and 6 deletions
+2
View File
@@ -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 add_menu_coloring()
fix potential buffer overflow in sym_val() fix potential buffer overflow in sym_val()
fix potential buffer overflow in pline(), raw_printf(), and config_error_add() 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 Fixes to Post-3.6.4 Problems that Were Exposed Via git Repository
+1
View File
@@ -1000,6 +1000,7 @@ int uid;
* print selected parts of score list. * print selected parts of score list.
* argc >= 2, with argv[0] untrustworthy (directory names, et al.), * argc >= 2, with argv[0] untrustworthy (directory names, et al.),
* and argv[1] starting with "-s". * and argv[1] starting with "-s".
* caveat: some shells might allow argv elements to be arbitrarily long.
*/ */
void void
prscore(argc, argv) prscore(argc, argv)
+18 -2
View File
@@ -243,7 +243,8 @@ void
choose_windows(s) choose_windows(s)
const char *s; const char *s;
{ {
register int i; int i;
char *tmps = 0;
for (i = 0; winchoices[i].procs; i++) { for (i = 0; winchoices[i].procs; i++) {
if ('+' == winchoices[i].procs->name[0]) if ('+' == winchoices[i].procs->name[0])
@@ -269,9 +270,22 @@ const char *s;
windowprocs.win_wait_synch = def_wait_synch; windowprocs.win_wait_synch = def_wait_synch;
if (!winchoices[0].procs) { if (!winchoices[0].procs) {
raw_printf("No window types?"); raw_printf("No window types supported?");
nh_terminate(EXIT_FAILURE); 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) { if (!winchoices[1].procs) {
config_error_add( config_error_add(
"Window type %s not recognized. The only choice is: %s", "Window type %s not recognized. The only choice is: %s",
@@ -293,6 +307,8 @@ const char *s;
config_error_add("Window type %s not recognized. Choices are: %s", config_error_add("Window type %s not recognized. Choices are: %s",
s, buf); s, buf);
} }
if (tmps)
free((genericptr_t) tmps) /*, tmps = 0*/ ;
if (windowprocs.win_raw_print == def_raw_print if (windowprocs.win_raw_print == def_raw_print
|| WINDOWPORT("safe-startup")) || WINDOWPORT("safe-startup"))
+4 -4
View File
@@ -355,6 +355,7 @@ char *argv[];
return 0; return 0;
} }
/* caveat: argv elements might be arbitrary long */
static void static void
process_options(argc, argv) process_options(argc, argv)
int argc; int argc;
@@ -383,11 +384,10 @@ char *argv[];
load_symset("DECGraphics", PRIMARY); load_symset("DECGraphics", PRIMARY);
switch_symbols(TRUE); switch_symbols(TRUE);
} else { } else {
raw_printf("Unknown option: %s", *argv); raw_printf("Unknown option: %.60s", *argv);
} }
break; break;
case 'X': case 'X':
discover = TRUE, wizard = FALSE; discover = TRUE, wizard = FALSE;
break; break;
#ifdef NEWS #ifdef NEWS
@@ -413,7 +413,7 @@ char *argv[];
load_symset("RogueIBM", ROGUESET); load_symset("RogueIBM", ROGUESET);
switch_symbols(TRUE); switch_symbols(TRUE);
} else { } else {
raw_printf("Unknown option: %s", *argv); raw_printf("Unknown option: %.60s", *argv);
} }
break; break;
case 'p': /* profession (role) */ case 'p': /* profession (role) */
@@ -451,7 +451,7 @@ char *argv[];
flags.initrole = i; flags.initrole = i;
break; break;
} }
/* else raw_printf("Unknown option: %s", *argv); */ /* else raw_printf("Unknown option: %.60s", *argv); */
} }
} }