summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authortobias <tobias@openbsd.org>2018-03-27 10:00:16 +0000
committertobias <tobias@openbsd.org>2018-03-27 10:00:16 +0000
commit76f6aa52a136c938240136b04de69a9bc9b2586e (patch)
tree861a2366b0ac63cea3bd549688d385122121f4c6
parentMake sure that programs violating a pledge(2) promise or some memory (diff)
downloadwireguard-openbsd-76f6aa52a136c938240136b04de69a9bc9b2586e.tar.xz
wireguard-openbsd-76f6aa52a136c938240136b04de69a9bc9b2586e.zip
Fix possibly wrong execution of commands and out of boundary write with
unusual input. Correction and regression tests based on FreeBSD bin/95079. While at it, fix another segmentation fault when using ' ' as magic character and also disallow '\0' as magic character. This cannot make any sense and avoids a theoretical out of boundary read. ok tb@
-rw-r--r--regress/usr.bin/apply/Makefile55
-rw-r--r--regress/usr.bin/apply/t1.in1
-rw-r--r--regress/usr.bin/apply/t1.out1
-rw-r--r--regress/usr.bin/apply/t2.out1
-rw-r--r--regress/usr.bin/apply/t3.out5
-rw-r--r--regress/usr.bin/apply/t4.out3
-rw-r--r--regress/usr.bin/apply/t5.out1
-rw-r--r--regress/usr.bin/apply/t6.out1
-rw-r--r--usr.bin/apply/apply.c108
9 files changed, 117 insertions, 59 deletions
diff --git a/regress/usr.bin/apply/Makefile b/regress/usr.bin/apply/Makefile
new file mode 100644
index 00000000000..d46325027f6
--- /dev/null
+++ b/regress/usr.bin/apply/Makefile
@@ -0,0 +1,55 @@
+# $OpenBSD: Makefile,v 1.1 2018/03/27 10:00:16 tobias Exp $
+
+APPLY?= apply
+CLEANFILES= *.res
+
+REGRESS_TARGETS=t1 t2 t3 t4 t5 t6
+
+# .in: input file
+# .out: desired output
+
+# t1: uses arguments multiple times (from FreeBSD bin/95079)
+# t2: overflows ARG_MAX (from FreeBSD bin/95079)
+# t3: debugs -0 call
+# t4: debugs -2 call
+# t5: uses magic character '&'
+# t6: uses magic character ' ' with command starting with a number
+
+t1:
+ @echo ${*}
+ @(${APPLY} "echo %1 %1 %1 %1" `cat ${*}.in` > ${*}.res)
+ @cmp -s ${*}.out ${.CURDIR}/${*}.res || \
+ (echo "XXX ${*} failed" && false)
+
+t2:
+ @echo ${*}
+ @ARG_MAX=`getconf ARG_MAX`;\
+ ARG_MAX_HALF=$$((ARG_MAX / 2)); \
+ ! ${APPLY} "echo %1 %1 %1" \
+ `jot $$ARG_MAX_HALF 1 1 | tr -d '\n'` > ${*}.res 2>&1
+
+t3:
+ @echo ${*}
+ @(${APPLY} -0 -d who 1 2 3 4 5 > ${*}.res)
+ @cmp -s ${*}.out ${.CURDIR}/${*}.res || \
+ (echo "XXX ${*} failed" && false)
+
+t4:
+ @echo ${*}
+ @(${APPLY} -2 -d cmp a1 b1 a2 b2 a3 b3 > ${*}.res)
+ @cmp -s ${*}.out ${.CURDIR}/${*}.res || \
+ (echo "XXX ${*} failed" && false)
+
+t5:
+ @echo ${*}
+ @(${APPLY} -a "&" "echo &2 &1" hello world > ${*}.res)
+ @cmp -s ${*}.out ${.CURDIR}/${*}.res || \
+ (echo "XXX ${*} failed" && false)
+
+t6:
+ @echo ${*}
+ @(${APPLY} -a " " -d "2to3 1" test.py > ${*}.res)
+ @cmp -s ${*}.out ${.CURDIR}/${*}.res || \
+ (echo "XXX ${*} failed" && false)
+
+.include <bsd.regress.mk>
diff --git a/regress/usr.bin/apply/t1.in b/regress/usr.bin/apply/t1.in
new file mode 100644
index 00000000000..8282be78243
--- /dev/null
+++ b/regress/usr.bin/apply/t1.in
@@ -0,0 +1 @@
+12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012
diff --git a/regress/usr.bin/apply/t1.out b/regress/usr.bin/apply/t1.out
new file mode 100644
index 00000000000..c725cb2fa98
--- /dev/null
+++ b/regress/usr.bin/apply/t1.out
@@ -0,0 +1 @@
+12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012
diff --git a/regress/usr.bin/apply/t2.out b/regress/usr.bin/apply/t2.out
new file mode 100644
index 00000000000..bcf5ab29339
--- /dev/null
+++ b/regress/usr.bin/apply/t2.out
@@ -0,0 +1 @@
+apply: Argument list too long
diff --git a/regress/usr.bin/apply/t3.out b/regress/usr.bin/apply/t3.out
new file mode 100644
index 00000000000..204caaf306e
--- /dev/null
+++ b/regress/usr.bin/apply/t3.out
@@ -0,0 +1,5 @@
+exec who
+exec who
+exec who
+exec who
+exec who
diff --git a/regress/usr.bin/apply/t4.out b/regress/usr.bin/apply/t4.out
new file mode 100644
index 00000000000..1cbec768894
--- /dev/null
+++ b/regress/usr.bin/apply/t4.out
@@ -0,0 +1,3 @@
+exec cmp a1 b1
+exec cmp a2 b2
+exec cmp a3 b3
diff --git a/regress/usr.bin/apply/t5.out b/regress/usr.bin/apply/t5.out
new file mode 100644
index 00000000000..b4a78f9c1e0
--- /dev/null
+++ b/regress/usr.bin/apply/t5.out
@@ -0,0 +1 @@
+world hello
diff --git a/regress/usr.bin/apply/t6.out b/regress/usr.bin/apply/t6.out
new file mode 100644
index 00000000000..9f014fb77a0
--- /dev/null
+++ b/regress/usr.bin/apply/t6.out
@@ -0,0 +1 @@
+exec 2to3 test.py
diff --git a/usr.bin/apply/apply.c b/usr.bin/apply/apply.c
index ee9e48162cb..f03f70d33ea 100644
--- a/usr.bin/apply/apply.c
+++ b/usr.bin/apply/apply.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: apply.c,v 1.27 2015/10/10 17:48:34 deraadt Exp $ */
+/* $OpenBSD: apply.c,v 1.28 2018/03/27 10:00:16 tobias Exp $ */
/* $NetBSD: apply.c,v 1.3 1995/03/25 03:38:23 glass Exp $ */
/*-
@@ -44,15 +44,42 @@
#include <string.h>
#include <unistd.h>
+#define ISMAGICNO(p) \
+ (p)[0] == magic && isdigit((unsigned char)(p)[1]) && (p)[1] != '0'
+
__dead void usage(void);
static int mysystem(const char *);
+char *str;
+size_t sz;
+
+void
+stradd(char *p)
+{
+ size_t n;
+
+ n = strlen(p);
+ if (str == NULL || sz - strlen(str) <= n) {
+ sz += (n / 1024 + 1) * 1024;
+ if ((str = realloc(str, sz)) == NULL)
+ err(1, "realloc");
+ }
+ strlcat(str, p, sz);
+}
+
+void
+strset(char *p)
+{
+ if (str != NULL)
+ str[0] = '\0';
+ stradd(p);
+}
+
int
main(int argc, char *argv[])
{
- int ch, clen, debug, i, l, magic, n, nargs, rval;
- char *c, *c2, *cmd, *p, *q;
- size_t len;
+ int ch, debug, i, magic, n, nargs, rval;
+ char buf[4], *cmd, *p;
if (pledge("stdio proc exec", NULL) == -1)
err(1, "pledge");
@@ -63,7 +90,7 @@ main(int argc, char *argv[])
while ((ch = getopt(argc, argv, "a:d0123456789")) != -1)
switch (ch) {
case 'a':
- if (optarg[1] != '\0')
+ if (optarg[0] == '\0' || optarg[1] != '\0')
errx(1,
"illegal magic character specification.");
magic = optarg[0];
@@ -93,8 +120,7 @@ main(int argc, char *argv[])
* largest one.
*/
for (n = 0, p = argv[0]; *p != '\0'; ++p)
- if (p[0] == magic &&
- isdigit((unsigned char)p[1]) && p[1] != '0') {
+ if (ISMAGICNO(p)) {
++p;
if (p[0] - '0' > n)
n = p[0] - '0';
@@ -106,28 +132,15 @@ main(int argc, char *argv[])
* the end to consume (nargs) arguments each time round the loop.
* Allocate enough space to hold the maximum command.
*/
+ strset(argv[0]);
if (n == 0) {
- len = sizeof("exec ") - 1 +
- strlen(argv[0]) + 9 * (sizeof(" %1") - 1) + 1;
- if ((cmd = malloc(len)) == NULL)
- err(1, NULL);
-
/* If nargs not set, default to a single argument. */
if (nargs == -1)
nargs = 1;
- l = snprintf(cmd, len, "exec %s", argv[0]);
- if (l >= len || l == -1)
- errx(1, "error building exec string");
- len -= l;
- p = cmd + l;
-
for (i = 1; i <= nargs; i++) {
- l = snprintf(p, len, " %c%d", magic, i);
- if (l >= len || l == -1)
- errx(1, "error numbering arguments");
- len -= l;
- p += l;
+ snprintf(buf, sizeof(buf), " %c%d", magic, i);
+ stradd(buf);
}
/*
@@ -136,19 +149,10 @@ main(int argc, char *argv[])
*/
if (nargs == 0)
nargs = 1;
- } else {
- if (asprintf(&cmd, "exec %s", argv[0]) == -1)
- err(1, NULL);
+ } else
nargs = n;
- }
-
- /*
- * Grab some space in which to build the command. Allocate
- * as necessary later, but no reason to build it up slowly
- * for the normal case.
- */
- if ((c = malloc(clen = 1024)) == NULL)
- err(1, NULL);
+ if ((cmd = strdup(str)) == NULL)
+ err(1, "strdup");
/*
* (argc) and (argv) are still offset by one to make it simpler to
@@ -156,35 +160,21 @@ main(int argc, char *argv[])
* equals 1 means that all the (argv) has been consumed.
*/
for (rval = 0; argc > nargs; argc -= nargs, argv += nargs) {
- /*
- * Find a max value for the command length, and ensure
- * there's enough space to build it.
- */
- for (l = strlen(cmd), i = 0; i < nargs; i++)
- l += strlen(argv[i+1]);
- if (l > clen) {
- if ((c2 = realloc(c, l)) == NULL)
- err(1, NULL);
- c = c2;
- clen = l;
- }
+ strset("exec ");
/* Expand command argv references. */
- for (p = cmd, q = c; *p != '\0'; ++p)
- if (p[0] == magic &&
- isdigit((unsigned char)p[1]) && p[1] != '0') {
- strlcpy(q, argv[(++p)[0] - '0'], c + clen - q);
- q += strlen(q);
- } else
- *q++ = *p;
-
- /* Terminate the command string. */
- *q = '\0';
+ for (p = cmd; *p != '\0'; ++p)
+ if (ISMAGICNO(p))
+ stradd(argv[*(++p) - '0']);
+ else {
+ strlcpy(buf, p, 2);
+ stradd(buf);
+ }
/* Run the command. */
if (debug)
- (void)printf("%s\n", c);
- else if (mysystem(c))
+ (void)printf("%s\n", str);
+ else if (mysystem(str))
rval = 1;
}