Our expr implementation supports 64 bit integers, but does not check
for overflows during parsing and arithmetical operations.
This patch fixes the problems based on FreeBSD's implementation, which
is a bit more advanced than NetBSD, which it is based upon.
The added regression test case is taken from NetBSD and passes with
this patch.
Okay?
Tobias
Index: regress/bin/Makefile
===================================================================
RCS file: /cvs/src/regress/bin/Makefile,v
retrieving revision 1.13
diff -u -p -u -p -r1.13 Makefile
--- regress/bin/Makefile 14 Jan 2018 22:04:47 -0000 1.13
+++ regress/bin/Makefile 30 Mar 2018 15:33:50 -0000
@@ -1,6 +1,6 @@
# $OpenBSD: Makefile,v 1.13 2018/01/14 22:04:47 bluhm Exp $
-SUBDIR+= cat chmod csh ed ksh ln md5 pax ps test
+SUBDIR+= cat chmod csh ed expr ksh ln md5 pax ps test
install:
Index: bin/expr/expr.c
===================================================================
RCS file: /cvs/src/bin/expr/expr.c,v
retrieving revision 1.26
diff -u -p -u -p -r1.26 expr.c
--- bin/expr/expr.c 19 Oct 2016 18:20:25 -0000 1.26
+++ bin/expr/expr.c 30 Mar 2018 15:33:50 -0000
@@ -99,6 +99,7 @@ is_integer(struct val *vp, int64_t *r)
char *s;
int neg;
int64_t i;
+ char c;
if (vp->type == integer) {
*r = vp->u.i;
@@ -120,8 +121,12 @@ is_integer(struct val *vp, int64_t *r)
if (!isdigit((unsigned char)*s))
return 0;
+ c = *s - '0';
+ if (c < 0 || INT64_MAX / 10 < i || INT64_MAX - c < i * 10)
+ errx(3, "out of range");
+
i *= 10;
- i += *s - '0';
+ i += c;
s++;
}
@@ -303,8 +308,9 @@ eval5(void)
struct val *
eval4(void)
{
- struct val *l, *r;
- enum token op;
+ struct val *l, *r;
+ enum token op;
+ volatile int64_t res;
l = eval5();
while ((op = token) == MUL || op == DIV || op == MOD) {
@@ -316,7 +322,10 @@ eval4(void)
}
if (op == MUL) {
- l->u.i *= r->u.i;
+ res = l->u.i * r->u.i;
+ if (r->u.i != 0 && l->u.i != res / r->u.i)
+ errx(3, "overflow");
+ l->u.i = res;
} else {
if (r->u.i == 0) {
errx(2, "division by zero");
@@ -324,6 +333,8 @@ eval4(void)
if (op == DIV) {
if (l->u.i != INT64_MIN || r->u.i != -1)
l->u.i /= r->u.i;
+ else
+ errx(3, "overflow");
} else {
if (l->u.i != INT64_MIN || r->u.i != -1)
l->u.i %= r->u.i;
@@ -342,8 +353,9 @@ eval4(void)
struct val *
eval3(void)
{
- struct val *l, *r;
- enum token op;
+ struct val *l, *r;
+ enum token op;
+ volatile int64_t res;
l = eval4();
while ((op = token) == ADD || op == SUB) {
@@ -355,9 +367,18 @@ eval3(void)
}
if (op == ADD) {
- l->u.i += r->u.i;
+ res = l->u.i + r->u.i;
+ if ((l->u.i > 0 && r->u.i > 0 && res <= 0) ||
+ (l->u.i < 0 && r->u.i < 0 && res >= 0))
+ errx(3, "overflow");
+ l->u.i = res;
} else {
- l->u.i -= r->u.i;
+ res = l->u.i - r->u.i;
+ if ((r->u.i == INT64_MIN && l->u.i < 0) ||
+ (l->u.i > 0 && r->u.i < 0 && res <= 0) ||
+ (l->u.i < 0 && r->u.i > 0 && res >= 0))
+ errx(3, "overflow");
+ l->u.i = res;
}
free_value(r);
Index: regress/bin/expr/Makefile
===================================================================
RCS file: regress/bin/expr/Makefile
diff -N regress/bin/expr/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ regress/bin/expr/Makefile 30 Mar 2018 15:33:50 -0000
@@ -0,0 +1,8 @@
+# $OpenBSD$
+
+REGRESS_TARGETS = expr
+
+expr:
+ sh ${.CURDIR}/expr.sh
+
+.include <bsd.regress.mk>
Index: regress/bin/expr/expr.sh
===================================================================
RCS file: regress/bin/expr/expr.sh
diff -N regress/bin/expr/expr.sh
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ regress/bin/expr/expr.sh 30 Mar 2018 15:33:50 -0000
@@ -0,0 +1,98 @@
+#!/bin/ksh
+
+: ${EXPR=expr}
+
+function test_expr {
+ #echo "Testing: `eval echo $1`"
+ res=`eval $EXPR $1 2>&1`
+ if [ "$res" != "$2" ]; then
+ echo "Expected $2, got $res from expression: `eval echo $1`"
+ exit 1
+ fi
+}
+
+# The first arg will get eval'd so escape any meta characters
+# The 2nd arg is an expected string/response from expr for that op.
+
+# Test overflow cases
+test_expr '4611686018427387904 + 4611686018427387903' '9223372036854775807'
+test_expr '4611686018427387904 + 4611686018427387904' "expr: overflow"
+test_expr '4611686018427387904 - -4611686018427387904' "expr: overflow"
+test_expr '-4611686018427387904 - 4611686018427387903' '-9223372036854775807'
+test_expr '-4611686018427387904 - 4611686018427387905' "expr: overflow"
+test_expr '-4611686018427387904 \* 1' '-4611686018427387904'
+test_expr '-4611686018427387904 \* -1' '4611686018427387904'
+test_expr '-4611686018427387904 \* 2' '-9223372036854775808'
+test_expr '-4611686018427387904 \* 3' "expr: overflow"
+test_expr '-4611686018427387904 \* -2' "expr: overflow"
+test_expr '4611686018427387904 \* 1' '4611686018427387904'
+test_expr '4611686018427387904 \* 2' "expr: overflow"
+test_expr '4611686018427387904 \* 3' "expr: overflow"
+
+# Test from gtk-- configure that cause problems on old expr
+test_expr '3 \> 3 \| 3 = 3 \& 4 \> 4 \| 3 = 3 \& 4 = 4 \& 5 \>= 5' '1'
+test_expr '3 \> 3 \| 3 = 3 \& 4 \> 4 \| 3 = 3 \& 4 = 4 \& 5 \>= 6' '0'
+test_expr '3 \> 3 \| 3 = 3 \& 4 \> 4 \| 3 = 3 \& 4 = 3 \& 5 \>= 5' '0'
+test_expr '3 \> 3 \| 3 = 3 \& 4 \> 4 \| 3 = 2 \& 4 = 4 \& 5 \>= 5' '0'
+test_expr '3 \> 2 \| 3 = 3 \& 4 \> 4 \| 3 = 3 \& 4 = 4 \& 5 \>= 6' '1'
+test_expr '3 \> 3 \| 3 = 3 \& 4 \> 3 \| 3 = 3 \& 4 = 4 \& 5 \>= 5' '1'
+
+# Basic precendence test with the : operator vs. math
+test_expr '2 : 4 / 2' '0'
+test_expr '4 : 4 % 3' '1'
+
+# Dangling arithmetic operator
+test_expr '.java_wrapper : /' '0'
+test_expr '4 : \*' '0'
+test_expr '4 : +' '0'
+test_expr '4 : -' '0'
+test_expr '4 : /' '0'
+test_expr '4 : %' '0'
+
+# Basic math test
+test_expr '2 + 4 \* 5' '22'
+
+# Basic functional tests
+test_expr '2' '2'
+test_expr '-4' '-4'
+test_expr 'hello' 'hello'
+
+# Compare operator precendence test
+test_expr '2 \> 1 \* 17' '0'
+
+# Compare operator tests
+test_expr '2 \!= 5' '1'
+test_expr '2 \!= 2' '0'
+test_expr '2 \<= 3' '1'
+test_expr '2 \<= 2' '1'
+test_expr '2 \<= 1' '0'
+test_expr '2 \< 3' '1'
+test_expr '2 \< 2' '0'
+test_expr '2 = 2' '1'
+test_expr '2 = 4' '0'
+test_expr '2 \>= 1' '1'
+test_expr '2 \>= 2' '1'
+test_expr '2 \>= 3' '0'
+test_expr '2 \> 1' '1'
+test_expr '2 \> 2' '0'
+
+# Known failure once
+test_expr '1 \* -1' '-1'
+
+# Test a known case that should fail
+test_expr '- 1 + 5' 'expr: syntax error'
+
+# Double check negative numbers
+test_expr '1 - -5' '6'
+
+# More complex math test for precedence
+test_expr '-3 + -1 \* 4 + 3 / -6' '-7'
+
+# The next two are messy but the shell escapes cause that.
+# Test precendence
+test_expr 'X1/2/3 : X\\\(.\*[^/]\\\)//\*[^/][^/]\*/\*$ \| . : \\\(.\\\)' '1/2'
+
+# Test proper () returning \1 from a regex
+test_expr '1/2 : .\*/\\\(.\*\\\)' '2'
+
+exit 0
