Skip site navigation (1)Skip section navigation (2)
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>