diff options
author | 2018-03-27 10:00:16 +0000 | |
---|---|---|
committer | 2018-03-27 10:00:16 +0000 | |
commit | 76f6aa52a136c938240136b04de69a9bc9b2586e (patch) | |
tree | 861a2366b0ac63cea3bd549688d385122121f4c6 | |
parent | Make sure that programs violating a pledge(2) promise or some memory (diff) | |
download | wireguard-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/Makefile | 55 | ||||
-rw-r--r-- | regress/usr.bin/apply/t1.in | 1 | ||||
-rw-r--r-- | regress/usr.bin/apply/t1.out | 1 | ||||
-rw-r--r-- | regress/usr.bin/apply/t2.out | 1 | ||||
-rw-r--r-- | regress/usr.bin/apply/t3.out | 5 | ||||
-rw-r--r-- | regress/usr.bin/apply/t4.out | 3 | ||||
-rw-r--r-- | regress/usr.bin/apply/t5.out | 1 | ||||
-rw-r--r-- | regress/usr.bin/apply/t6.out | 1 | ||||
-rw-r--r-- | usr.bin/apply/apply.c | 108 |
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; } |