From f460047983e650b784d89a364415d3d21650bd61 Mon Sep 17 00:00:00 2001
From: PatR <rankin@nethack.org>
Date: Wed, 20 Sep 2017 18:52:53 -0700
Subject: [PATCH] '#' command fix for 'extmenu'
While doing some cleanup I found an old personal bug list with four
entries. Two have already been fixed, or at least I couldn't reproduce
them with current code, one is still pending (dungeon overview feedback
is inconsistent if you find an unlit temple and haven't seen its altar
yet), plus this one: a buffer overflow (triggering a crash for me) in
wizard mode if you turn on the 'extmenu' option and start an extended
command. The menu can't handle long line width for 'w' with all its
wizthis and wizthat entries; strcat() goes out of bounds writing into
a local array.
(This bug predates the keybinding patch that turned all commands into
extended commands.)
---
doc/fixes36.1 | 1 +
src/cmd.c | 55 ++++++++++++++++++++++++++++++---------------------
2 files changed, 34 insertions(+), 22 deletions(-)
diff --git a/doc/fixes36.1 b/doc/fixes36.1
index 7379669ab..968b720fe 100644
--- a/doc/fixes36.1
+++ b/doc/fixes36.1
@@ -439,6 +439,7 @@ if leash or unleash attempt was directed at "remembered, unseen monster" glyph
and yielded "there's no creature there", the glyph wasn't removed
wizmode level teleport menu indicates on what level you currently are
fix invisible gold symbol in status line when S_coin was set to space
+fix buffer overflow in wizard mode for '#' command when 'extmenu' option is on
Fixes to Post-3.6.0 Problems that Were Exposed Via git Repository
diff --git a/src/cmd.c b/src/cmd.c
index 9ea689844..6849f2ca4 100644
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -381,8 +381,10 @@ doextlist(VOID_ARGS)
#define MAX_EXT_CMD 200 /* Change if we ever have more ext cmds */
/*
- * This is currently used only by the tty port and is
- * controlled via runtime option 'extmenu'.
+ * This is currently used only by the tty interface and is
+ * controlled via runtime option 'extmenu'. (Most other interfaces
+ * already use a menu all the time for extended commands.)
+ *
* ``# ?'' is counted towards the limit of the number of commands,
* so we actually support MAX_EXT_CMD-1 "real" extended commands.
*
@@ -399,9 +401,10 @@ extcmd_via_menu()
char buf[BUFSZ];
char cbuf[QBUFSZ], prompt[QBUFSZ], fmtstr[20];
int i, n, nchoices, acount;
- int ret, biggest;
+ int ret, len, biggest;
int accelerator, prevaccelerator;
int matchlevel = 0;
+ boolean wastoolong, one_per_line;
ret = 0;
cbuf[0] = '\0';
@@ -416,10 +419,8 @@ extcmd_via_menu()
continue;
if (!matchlevel || !strncmp(efp->ef_txt, cbuf, matchlevel)) {
choices[i] = efp;
- if ((int) strlen(efp->ef_desc) > biggest) {
- biggest = strlen(efp->ef_desc);
- Sprintf(fmtstr, "%%-%ds", biggest + 15);
- }
+ if ((len = (int) strlen(efp->ef_desc)) > biggest)
+ biggest = len;
if (++i > MAX_EXT_CMD) {
#if defined(BETA)
impossible(
@@ -434,25 +435,33 @@ extcmd_via_menu()
choices[i] = (struct ext_func_tab *) 0;
nchoices = i;
/* if we're down to one, we have our selection so get out of here */
- if (nchoices == 1) {
- for (i = 0; extcmdlist[i].ef_txt != (char *) 0; i++)
- if ((extcmdlist[i].flags & AUTOCOMPLETE)
- && !(!wizard && (extcmdlist[i].flags & WIZMODECMD))
- && !strncmpi(extcmdlist[i].ef_txt, cbuf, matchlevel)) {
- ret = i;
- break;
- }
+ if (nchoices <= 1) {
+ ret = (nchoices == 1) ? (int) (choices[0] - extcmdlist) : -1;
break;
}
/* otherwise... */
win = create_nhwindow(NHW_MENU);
start_menu(win);
+ Sprintf(fmtstr, "%%-%ds", biggest + 15);
+ prompt[0] = '\0';
+ wastoolong = FALSE; /* True => had to wrap due to line width
+ * ('w' in wizard mode) */
+ /* -3: two line menu header, 1 line menu footer (for prompt) */
+ one_per_line = (nchoices < ROWNO - 3);
accelerator = prevaccelerator = 0;
acount = 0;
for (i = 0; choices[i]; ++i) {
accelerator = choices[i]->ef_txt[matchlevel];
- if (accelerator != prevaccelerator || nchoices < (ROWNO - 3)) {
+ if (accelerator != prevaccelerator || one_per_line)
+ wastoolong = FALSE;
+ if (accelerator != prevaccelerator || one_per_line
+ || (acount >= 2
+ /* +4: + sizeof " or " - sizeof "" */
+ && (strlen(prompt) + 4 + strlen(choices[i]->ef_txt)
+ /* -6: enough room for 1 space left margin
+ * + "%c - " menu selector + 1 space right margin */
+ >= min(sizeof prompt, COLNO - 6)))) {
if (acount) {
/* flush extended cmds for that letter already in buf */
Sprintf(buf, fmtstr, prompt);
@@ -460,15 +469,17 @@ extcmd_via_menu()
add_menu(win, NO_GLYPH, &any, any.a_char, 0, ATR_NONE,
buf, FALSE);
acount = 0;
+ if (!(accelerator != prevaccelerator || one_per_line))
+ wastoolong = TRUE;
}
}
prevaccelerator = accelerator;
- if (!acount || nchoices < (ROWNO - 3)) {
- Sprintf(prompt, "%s [%s]", choices[i]->ef_txt,
- choices[i]->ef_desc);
+ if (!acount || one_per_line) {
+ Sprintf(prompt, "%s%s [%s]", wastoolong ? "or " : "",
+ choices[i]->ef_txt, choices[i]->ef_desc);
} else if (acount == 1) {
- Sprintf(prompt, "%s or %s", choices[i - 1]->ef_txt,
- choices[i]->ef_txt);
+ Sprintf(prompt, "%s%s or %s", wastoolong ? "or " : "",
+ choices[i - 1]->ef_txt, choices[i]->ef_txt);
} else {
Strcat(prompt, " or ");
Strcat(prompt, choices[i]->ef_txt);
@@ -2858,7 +2869,7 @@ int final;
struct ext_func_tab extcmdlist[] = {
{ '#', "#", "perform an extended command",
doextcmd, IFBURIED | GENERALCMD },
- { M('?'), "?", "get this list of extended commands",
+ { M('?'), "?", "list all extended commands",
doextlist, IFBURIED | AUTOCOMPLETE | GENERALCMD },
{ M('a'), "adjust", "adjust inventory letters",
doorganize, IFBURIED | AUTOCOMPLETE },
--
2.40.0