Merge pull request #6120 from MathiasVP/not-overflow-is-barrier-in-cwe-190

C++: Recognize any non-overflowing arithmetic expression as a barrier for `cpp/uncontrolled-arithmetic`

Author: geoffw0

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())

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
+}