Control: tags -1 + patch
I grepped for MAXARGS and arranged for screen to crash rather than
silently ignoring arguments. I think this is much better, although it
doesn't fully fix the bug.
Confusingly, the source uses MAXARGS both for the maximum number of
arguments for a shell command, and for the maximum number of numerical
arguments to a terminal escape sequence. I left the latter part
alone: excess terminal escape sequence arguments are still ignored.
Applying this patch wouldn't actually fix the bug but it would IMO
render it of "normal" or "minor" severity.
Ian.
>From c73ab91cc30ac9dbc38df54f5eee47e8676fa453 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijack...@chiark.greenend.org.uk>
Date: Thu, 18 Nov 2021 13:55:19 +0000
Subject: [PATCH] crash rather than silently dropping arguments
---
attacher.c | 10 +++++++---
extern.h | 1 +
screen.c | 6 ++++++
socket.c | 14 +++++++++-----
4 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/attacher.c b/attacher.c
index 18ba43c..23ed479 100644
--- a/attacher.c
+++ b/attacher.c
@@ -1042,11 +1042,15 @@ int query;
}
p = m.m.command.cmd;
n = 0;
- for (; *av && n < MAXARGS - 1; ++av, ++n)
+ for (; *av; ++av, ++n)
{
+ if (!(n < MAXARGS - 1)) {
+ Crash_due_to_argument_limit();
+ }
len = strlen(*av) + 1;
- if (p + len >= m.m.command.cmd + sizeof(m.m.command.cmd) - 1)
- break;
+ if (p + len >= m.m.command.cmd + sizeof(m.m.command.cmd) - 1) {
+ Crash_due_to_argument_limit();
+ }
strcpy(p, *av);
p += len;
}
diff --git a/extern.h b/extern.h
index 8374ec0..74483d6 100644
--- a/extern.h
+++ b/extern.h
@@ -52,6 +52,7 @@ extern void Panic __P(());
extern void QueryMsg __P(());
extern void Dummy __P(());
#endif
+extern void Crash_due_to_argument_limit __P(());
extern void Finit __P((int));
extern void MakeNewEnv __P((void));
extern char *MakeWinMsg __P((char *, struct win *, int));
diff --git a/screen.c b/screen.c
index 1ab7ae1..c5b9653 100644
--- a/screen.c
+++ b/screen.c
@@ -2169,6 +2169,12 @@ DEFINE_VARARGS_FN(Panic)
eexit(1);
}
+void Crash_due_to_argument_limit()
+{
+ Panic(0, "Argument count or length limit exceeded");
+}
+
+
DEFINE_VARARGS_FN(QueryMsg)
{
char buf[MAXPATHLEN*2];
diff --git a/socket.c b/socket.c
index 01674c1..2a8545f 100644
--- a/socket.c
+++ b/socket.c
@@ -701,11 +701,15 @@ struct NewWindow *nwin;
p = m.m.create.line;
n = 0;
if (nwin->args != nwin_undef.args)
- for (av = nwin->args; *av && n < MAXARGS - 1; ++av, ++n)
+ for (av = nwin->args; *av; ++av, ++n)
{
+ if (!(n < MAXARGS - 1)) {
+ Crash_due_to_argument_limit();
+ }
len = strlen(*av) + 1;
- if (p + len >= m.m.create.line + sizeof(m.m.create.line) - 1)
- break;
+ if (p + len >= m.m.create.line + sizeof(m.m.create.line) - 1) {
+ Crash_due_to_argument_limit();
+ }
strcpy(p, *av);
p += len;
}
@@ -773,7 +777,7 @@ struct msg *mp;
nwin = nwin_undef;
n = mp->m.create.nargs;
if (n > MAXARGS - 1)
- n = MAXARGS - 1;
+ Crash_due_to_argument_limit();
/* ugly hack alert... should be done by the frontend! */
if (n)
{
@@ -1782,7 +1786,7 @@ struct msg *mp;
n = mp->m.command.nargs;
if (n > MAXARGS - 1)
- n = MAXARGS - 1;
+ Crash_due_to_argument_limit();
for (fc = fullcmd; n > 0; n--)
{
int len = strlen(p);
--
2.20.1
--
Ian Jackson <ijack...@chiark.greenend.org.uk> These opinions are my own.
Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.
