From 44dcf95e04f9f636ddbc0dcc1effe5f56bc37a20 Mon Sep 17 00:00:00 2001 From: Michael Meyer Date: Thu, 29 Sep 2022 20:22:00 -0400 Subject: [PATCH] Fix: X11 extcmd menu heap buffer overflow Instead of allocating space for ((n + 1) * size) (to make room for all entries plus terminator), it allocated space for (n * size + 1). init_extended_commands_popup would therefore write past the end of the memory allocated for command_list and command_indx when trying to store their respective terminator entries. This meant the X11 windowport would crash when accessing the extended command menu, if NetHack had been compiled with -fsanitize=address. I also did some minor cleanup/refactoring to eliminate variables and lines that were made redundant or useless by 9d64d13, which changed the way the function worked and removed the need for things like tracking indices in the source and destination arrays with separate variables. --- win/X11/winmisc.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/win/X11/winmisc.c b/win/X11/winmisc.c index b411510a6..64284d005 100644 --- a/win/X11/winmisc.c +++ b/win/X11/winmisc.c @@ -1885,7 +1885,7 @@ ec_key(Widget w, XEvent *event, String *params, Cardinal *num_params) static void init_extended_commands_popup(void) { - int i, j, num_commands; + int i, num_commands; int *matches; int ecmflags = ECM_NO1CHARCMD; @@ -1894,19 +1894,18 @@ init_extended_commands_popup(void) num_commands = extcmds_match(NULL, ecmflags, &matches); - j = num_commands; - command_list = (const char **) alloc((unsigned) (j * sizeof (char *) + 1)); - command_indx = (short *) alloc((unsigned) (j * sizeof (short) + 1)); + i = num_commands + 1; /* room for each extcmd, plus terminator */ + command_list = (const char **) alloc((unsigned) (i * sizeof(char *))); + command_indx = (short *) alloc((unsigned) (i * sizeof(short))); - for (i = j = 0; i < num_commands; i++) { + for (i = 0; i < num_commands; i++) { struct ext_func_tab *ec = extcmds_getentry(matches[i]); - command_indx[j] = matches[i]; - command_list[j++] = ec->ef_txt; + command_indx[i] = matches[i]; + command_list[i] = ec->ef_txt; } - command_list[j] = (char *) 0; - command_indx[j] = -1; - num_commands = j; + command_list[i] = (char *) 0; + command_indx[i] = -1; extended_command_popup = make_menu("extended_commands", "Extended Commands", -- 2.49.0