diff --git a/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql b/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql index 359ac7a0d1a0..5bd9d3244021 100644 --- a/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql +++ b/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql @@ -36,7 +36,7 @@ predicate boundedDiv(Expr e, Expr left) { e = left } /** * An operand `e` of a remainder expression `rem` (i.e., `rem` is either a `RemExpr` or - * an `AssignRemExpr`) with left-hand side `left` and right-ahnd side `right` is bounded + * an `AssignRemExpr`) with left-hand side `left` and right-hand side `right` is bounded * when `e` is `left` and `right` is upper bounded by some number that is less than the maximum integer * allowed by the result type of `rem`. */ @@ -59,10 +59,17 @@ predicate boundedBitwiseAnd(Expr e, Expr andExpr, Expr operand1, Expr operand2) } /** - * Holds if `fc` is a part of the left operand of a binary operation that greatly reduces the range - * of possible values. + * Holds if `e` is an arithmetic expression that cannot overflow, or if `e` is an operand of an + * operation that may greatly reduces the range of possible values. */ predicate bounded(Expr e) { + ( + e instanceof UnaryArithmeticOperation or + e instanceof BinaryArithmeticOperation or + e instanceof AssignArithmeticOperation + ) and + not convertedExprMightOverflow(e) + or // For `%` and `&` we require that `e` is bounded by a value that is strictly smaller than the // maximum possible value of the result type of the operation. // For example, the function call `rand()` is considered bounded in the following program: @@ -85,7 +92,7 @@ predicate bounded(Expr e) { boundedBitwiseAnd(e, andExpr, andExpr.getAnOperand(), andExpr.getAnOperand()) ) or - // Optimitically assume that a division always yields a much smaller value. + // Optimitically assume that a division or right shift always yields a much smaller value. boundedDiv(e, any(DivExpr div).getLeftOperand()) or boundedDiv(e, any(AssignDivExpr div).getLValue()) diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/uncontrolled/test.c b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/uncontrolled/test.c index 61f39a8e8519..127266f2a6a8 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/uncontrolled/test.c +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/uncontrolled/test.c @@ -3,12 +3,12 @@ int rand(void); void trySlice(int start, int end); +void add_100(int); #define RAND() rand() #define RANDN(n) (rand() % n) #define RAND2() (rand() ^ rand()) - - +#define RAND_MAX 32767 @@ -99,4 +99,14 @@ void randomTester() { *ptr_r = RAND(); r -= 100; // BAD } + + { + int r = rand(); + r = ((2.0 / (RAND_MAX + 1)) * r - 1.0); + add_100(r); + } } + +void add_100(int r) { + r += 100; // GOOD +} \ No newline at end of file