Date: Mon, 5 Aug 2002 13:41:20 +0400 (MSD) From: Maxim Konovalov <maxim@FreeBSD.org> To: audit@FreeBSD.org Subject: test(1) diff for review Message-ID: <20020805132815.V60089-100000@news1.macomnet.ru>
next in thread | raw e-mail | index | archive | help
Hello -audit, At the present time test(1) parses argv[] array quite liberal. It works for standalone test(1) but when test(1) was integrated in sh(1) the latter started fails. There was an attempt to work around the problem in rev. 1.40 but it introduced a memory leak (see bin/40117 for details) which I tried to fix in rev. 1.49. As Stefan Farfeleder <e0026813@stud3.tuwien.ac.at> noted my fix is incomplete. Here is a more comprehensive patch for test(1): o Backout rev. 1.40 and rev. 1.49. o Add argv[] boudary check. Tested on -stable and -current. The diff is against -current. Index: test.c =================================================================== RCS file: /home/ncvs/src/bin/test/test.c,v retrieving revision 1.49 diff -u -r1.49 test.c --- test.c 5 Jul 2002 10:27:34 -0000 1.49 +++ test.c 29 Jul 2002 07:03:46 -0000 @@ -161,6 +161,7 @@ }; struct t_op const *t_wp_op; +int nargc; char **t_wp; static int aexpr(enum token); @@ -182,23 +183,8 @@ int main(int argc, char **argv) { - int i, res; + int res; char *p; - char **nargv; - - /* - * XXX copy the whole contents of argv to a newly allocated - * space with two extra cells filled with NULL's - this source - * code totally depends on their presence. - */ - if ((nargv = (char **)malloc((argc + 2) * sizeof(char *))) == NULL) - error("Out of space"); - - for (i = 0; i < argc; i++) - nargv[i] = argv[i]; - - nargv[i] = nargv[i + 1] = NULL; - argv = nargv; if ((p = rindex(argv[0], '/')) == NULL) p = argv[0]; @@ -210,15 +196,19 @@ argv[argc] = NULL; } + /* no expression => false */ + if (--argc <= 0) + return 1; + #ifndef SHELL (void)setlocale(LC_CTYPE, ""); #endif + nargc = argc; t_wp = &argv[1]; res = !oexpr(t_lex(*t_wp)); - if (*t_wp != NULL && *++t_wp != NULL) + if (--nargc > 0) syntax(*t_wp, "unexpected operator"); - free(nargv); return res; } @@ -239,9 +229,11 @@ int res; res = aexpr(n); - if (t_lex(*++t_wp) == BOR) - return oexpr(t_lex(*++t_wp)) || res; + if (t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL) == BOR) + return oexpr(t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL)) || + res; t_wp--; + nargc++; return res; } @@ -251,9 +243,11 @@ int res; res = nexpr(n); - if (t_lex(*++t_wp) == BAND) - return aexpr(t_lex(*++t_wp)) && res; + if (t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL) == BAND) + return aexpr(t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL)) && + res; t_wp--; + nargc++; return res; } @@ -261,7 +255,7 @@ nexpr(enum token n) { if (n == UNOT) - return !nexpr(t_lex(*++t_wp)); + return !nexpr(t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL)); return primary(n); } @@ -274,30 +268,32 @@ if (n == EOI) return 0; /* missing expression */ if (n == LPAREN) { - if ((nn = t_lex(*++t_wp)) == RPAREN) + if ((nn = t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL)) == + RPAREN) return 0; /* missing expression */ res = oexpr(nn); - if (t_lex(*++t_wp) != RPAREN) + if (t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL) != RPAREN) syntax(NULL, "closing paren expected"); return res; } if (t_wp_op && t_wp_op->op_type == UNOP) { /* unary expression */ - if (*++t_wp == NULL) + if (--nargc == 0) syntax(t_wp_op->op_text, "argument expected"); switch (n) { case STREZ: - return strlen(*t_wp) == 0; + return strlen(*++t_wp) == 0; case STRNZ: - return strlen(*t_wp) != 0; + return strlen(*++t_wp) != 0; case FILTT: - return isatty(getn(*t_wp)); + return isatty(getn(*++t_wp)); default: - return filstat(*t_wp, n); + return filstat(*++t_wp, n); } } - if (t_lex(t_wp[1]), t_wp_op && t_wp_op->op_type == BINOP) { + if (t_lex(nargc > 0 ? t_wp[1] : NULL), t_wp_op && t_wp_op->op_type == + BINOP) { return binop(); } @@ -311,10 +307,10 @@ struct t_op const *op; opnd1 = *t_wp; - (void) t_lex(*++t_wp); + (void) t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL); op = t_wp_op; - if ((opnd2 = *++t_wp) == NULL) + if ((opnd2 = nargc > 0 ? (--nargc, *++t_wp) : NULL) == NULL) syntax(op->op_text, "argument expected"); switch (op->op_num) { @@ -415,7 +411,7 @@ while (op->op_text) { if (strcmp(s, op->op_text) == 0) { if ((op->op_type == UNOP && isoperand()) || - (op->op_num == LPAREN && *(t_wp+1) == 0)) + (op->op_num == LPAREN && nargc == 1)) break; t_wp_op = op; return op->op_num; @@ -433,10 +429,12 @@ char *s; char *t; - if ((s = *(t_wp+1)) == 0) + if (nargc == 1) return 1; - if ((t = *(t_wp+2)) == 0) + if (nargc == 2) return 0; + s = *(t_wp + 1); + t = *(t_wp + 2); while (op->op_text) { if (strcmp(s, op->op_text) == 0) return op->op_type == BINOP && %%% -- Maxim Konovalov, maxim@FreeBSD.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020805132815.V60089-100000>