Module Name: src Committed By: kamil Date: Tue Jun 12 18:12:18 UTC 2018
Modified Files: src/bin/expr: expr.y Log Message: 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> To generate a diff of this commit: cvs rdiff -u -r1.39 -r1.40 src/bin/expr/expr.y Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/bin/expr/expr.y diff -u src/bin/expr/expr.y:1.39 src/bin/expr/expr.y:1.40 --- src/bin/expr/expr.y:1.39 Mon Sep 5 01:00:07 2016 +++ src/bin/expr/expr.y Tue Jun 12 18:12:18 2018 @@ -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. @@ -32,7 +32,7 @@ %{ #include <sys/cdefs.h> #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 */ #include <sys/types.h> @@ -273,8 +273,7 @@ is_integer(const char *str) static int64_t perform_arith_op(const char *left, const char *op, const char *right) { - int64_t res, sign, l, r; - u_int64_t temp; + int64_t res, l, r; res = 0; @@ -307,66 +306,68 @@ perform_arith_op(const char *left, const switch(op[0]) { case '+': - /* - * Do the op into an unsigned to avoid overflow and then cast - * back to check the resulting signage. + /* + * Check for over-& underflow. */ - temp = l + r; - res = (int64_t) temp; - /* very simplistic check for over-& underflow */ - if ((res < 0 && l > 0 && r > 0) - || (res > 0 && l < 0 && r < 0)) + if ((l > 0 && r <= INT64_MAX - l) || + (l < 0 && r >= INT64_MIN - l)) { + res = l + r; + } else { yyerror("integer overflow or underflow occurred for " "operation '%s %s %s'", left, op, right); + } break; case '-': - /* - * Do the op into an unsigned to avoid overflow and then cast - * back to check the resulting signage. + /* + * Check for over-& underflow. */ - temp = l - r; - res = (int64_t) temp; - /* very simplistic check for over-& underflow */ - if ((res < 0 && l > 0 && l > r) - || (res > 0 && l < 0 && l < r) ) + if ((r > 0 && l < INT64_MIN + r) || + (r < 0 && l > INT64_MAX + r)) { yyerror("integer overflow or underflow occurred for " "operation '%s %s %s'", left, op, right); + } else { + res = l - r; + } break; case '/': - if (r == 0) + if (r == 0) 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; break; case '%': if (r == 0) 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; break; case '*': - /* shortcut */ - if ((l == 0) || (r == 0)) { - res = 0; - break; - } - - sign = 1; - if (l < 0) - sign *= -1; - if (r < 0) - sign *= -1; - - 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. + * Check for over-& underflow. */ - if ((res < 0 && sign > 0) || (res > 0 && sign < 0) || - (res == 0)) + + /* Simplify the conditions */ + if (l < 0 && r < 0 && l != INT64_MIN && r != INT64_MIN) { + l = -l; + r = -r; + } + + if ((l < 0 && r < 0) || + ((l != 0 && r != 0) && + (((l > 0 && r > 0) && (l > INT64_MAX / r)) || + ((((l < 0 && r > 0) || (l > 0 && r < 0)) && + (r != -1 && (l < INT64_MIN / r))))))) { yyerror("integer overflow or underflow occurred for " "operation '%s %s %s'", left, op, right); /* NOTREACHED */ + } else { + res = l * r; + } break; } return res;