Aren
/
NetBSD
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rework perform_arith_op() in expr(1) to omit Undefined Behavior 

The current implementation of operations - + * / % could cause Undefined
Behavior and in narrow cases (INT64_MIN / -1 and INT64_MIN % -1) SIGFPE
and crash duping core.

Detected with MKSANITIZER enabled for the Undefined Behavior variation:
# eval expr '4611686018427387904 + 4611686018427387904'
/public/src.git/bin/expr/expr.y:315:12: runtime error: signed integer overflow: 4611686018427387904 + 4611686018427387904 cannot be represented in type 'long'

All bin/t_expr ATF tests pass now in a sanitized userland.

Sponsored by <The NetBSD Foundation>
This commit is contained in:
kamil 2018-06-12 18:12:18 +00:00
parent 88efda49f6
commit 7806b47917
1 changed files with 40 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
bin/expr/expr.y
View File

@ -1,4 +1,4 @@
/* $NetBSD: expr.y,v 1.39 2016/09/05 01:00:07 sevan Exp $ */ /* $NetBSD: expr.y,v 1.40 2018/06/12 18:12:18 kamil Exp $ */
/*_ /*_
* Copyright (c) 2000 The NetBSD Foundation, Inc. * Copyright (c) 2000 The NetBSD Foundation, Inc.
@ -32,7 +32,7 @@
%{ %{
#include <sys/cdefs.h> #include <sys/cdefs.h>
#ifndef lint #ifndef lint
__RCSID("$NetBSD: expr.y,v 1.39 2016/09/05 01:00:07 sevan Exp $"); __RCSID("$NetBSD: expr.y,v 1.40 2018/06/12 18:12:18 kamil Exp $");
#endif /* not lint */ #endif /* not lint */
#include <sys/types.h> #include <sys/types.h>
@ -273,8 +273,7 @@ is_integer(const char *str)
static int64_t static int64_t
perform_arith_op(const char *left, const char *op, const char *right) perform_arith_op(const char *left, const char *op, const char *right)
{ {
int64_t res, sign, l, r; int64_t res, l, r;
u_int64_t temp;
res = 0; res = 0;
@ -308,65 +307,67 @@ perform_arith_op(const char *left, const char *op, const char *right)
switch(op[0]) { switch(op[0]) {
case '+': case '+':
/* /*
* Do the op into an unsigned to avoid overflow and then cast * Check for over-& underflow.
* back to check the resulting signage.
*/ */
temp = l + r; if ((l > 0 && r <= INT64_MAX - l) ||
res = (int64_t) temp; (l < 0 && r >= INT64_MIN - l)) {
/* very simplistic check for over-& underflow */ res = l + r;
if ((res < 0 && l > 0 && r > 0) } else {
|| (res > 0 && l < 0 && r < 0))
yyerror("integer overflow or underflow occurred for " yyerror("integer overflow or underflow occurred for "
"operation '%s %s %s'", left, op, right); "operation '%s %s %s'", left, op, right);
}
break; break;
case '-': case '-':
/* /*
* Do the op into an unsigned to avoid overflow and then cast * Check for over-& underflow.
* back to check the resulting signage.
*/ */
temp = l - r; if ((r > 0 && l < INT64_MIN + r) ||
res = (int64_t) temp; (r < 0 && l > INT64_MAX + r)) {
/* very simplistic check for over-& underflow */
if ((res < 0 && l > 0 && l > r)
|| (res > 0 && l < 0 && l < r) )
yyerror("integer overflow or underflow occurred for " yyerror("integer overflow or underflow occurred for "
"operation '%s %s %s'", left, op, right); "operation '%s %s %s'", left, op, right);
} else {
res = l - r;
}
break; break;
case '/': case '/':
if (r == 0) if (r == 0)
yyerror("second argument to '%s' must not be zero", op); yyerror("second argument to '%s' must not be zero", op);
if (l == INT64_MIN && r == -1)
yyerror("integer overflow or underflow occurred for "
"operation '%s %s %s'", left, op, right);
res = l / r; res = l / r;
break; break;
case '%': case '%':
if (r == 0) if (r == 0)
yyerror("second argument to '%s' must not be zero", op); yyerror("second argument to '%s' must not be zero", op);
if (l == INT64_MIN && r == -1)
yyerror("integer overflow or underflow occurred for "
"operation '%s %s %s'", left, op, right);
res = l % r; res = l % r;
break; break;
case '*': case '*':
/* shortcut */ /*
if ((l == 0) || (r == 0)) { * Check for over-& underflow.
res = 0; */
break;
/* Simplify the conditions */
if (l < 0 && r < 0 && l != INT64_MIN && r != INT64_MIN) {
l = -l;
r = -r;
} }
sign = 1; if ((l < 0 && r < 0) ||
if (l < 0) ((l != 0 && r != 0) &&
sign *= -1; (((l > 0 && r > 0) && (l > INT64_MAX / r)) ||
if (r < 0) ((((l < 0 && r > 0) || (l > 0 && r < 0)) &&
sign *= -1; (r != -1 && (l < INT64_MIN / r))))))) {
res = l * r;
/*
* XXX: not the most portable but works on anything with 2's
* complement arithmetic. If the signs don't match or the
* result was 0 on 2's complement this overflowed.
*/
if ((res < 0 && sign > 0) || (res > 0 && sign < 0) ||
(res == 0))
yyerror("integer overflow or underflow occurred for " yyerror("integer overflow or underflow occurred for "
"operation '%s %s %s'", left, op, right); "operation '%s %s %s'", left, op, right);
/* NOTREACHED */ /* NOTREACHED */
} else {
res = l * r;
}
break; break;
} }
return res; return res;