Merge pull request #7459 from MathiasVP/promote-arithmetic-uncontrolled

C++: Increase precision of `cpp/arithmetic-uncontrolled` to `high`

cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql

@@ -5,7 +5,7 @@
  * @kind path-problem
  * @problem.severity warning
  * @security-severity 8.6
- * @precision medium
+ * @precision high
  * @id cpp/uncontrolled-arithmetic
  * @tags security
  *       external/cwe/cwe-190
@@ -82,8 +82,11 @@ predicate missingGuard(VariableAccess va, string effect) {
     op.getUnspecifiedType().(IntegralType).isUnsigned() and
     not op instanceof MulExpr
     or
-    // overflow
-    missingGuardAgainstOverflow(op, va) and effect = "overflow"
+    // overflow - only report signed integer overflow since unsigned overflow
+    // is well-defined.
+    op.getUnspecifiedType().(IntegralType).isSigned() and
+    missingGuardAgainstOverflow(op, va) and
+    effect = "overflow"
   )
 }
 

cpp/ql/src/change-notes/2022-01-05-promote-uncontrolled-arithmetic.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* The "Uncontrolled data in arithmetic expression" (cpp/uncontrolled-arithmetic) query has been enhanced to reduce false positive results and its @precision increased to high.

cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected

@@ -35,8 +35,6 @@ edges
 | test.cpp:190:10:190:13 | call to rand | test.cpp:205:7:205:7 | y |
 | test.cpp:190:10:190:13 | call to rand | test.cpp:208:7:208:7 | y |
 | test.cpp:215:11:215:14 | call to rand | test.cpp:219:8:219:8 | x |
-| test.cpp:223:20:223:23 | call to rand | test.cpp:227:8:227:8 | x |
-| test.cpp:223:20:223:25 | (unsigned int)... | test.cpp:227:8:227:8 | x |
 nodes
 | test.c:18:13:18:16 | call to rand | semmle.label | call to rand |
 | test.c:21:17:21:17 | r | semmle.label | r |
@@ -92,9 +90,6 @@ nodes
 | test.cpp:208:7:208:7 | y | semmle.label | y |
 | test.cpp:215:11:215:14 | call to rand | semmle.label | call to rand |
 | test.cpp:219:8:219:8 | x | semmle.label | x |
-| test.cpp:223:20:223:23 | call to rand | semmle.label | call to rand |
-| test.cpp:223:20:223:25 | (unsigned int)... | semmle.label | (unsigned int)... |
-| test.cpp:227:8:227:8 | x | semmle.label | x |
 subpaths
 #select
 | test.c:21:17:21:17 | r | test.c:18:13:18:16 | call to rand | test.c:21:17:21:17 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:18:13:18:16 | call to rand | Uncontrolled value |
@@ -125,5 +120,3 @@ subpaths
 | test.cpp:205:7:205:7 | y | test.cpp:190:10:190:13 | call to rand | test.cpp:205:7:205:7 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:190:10:190:13 | call to rand | Uncontrolled value |
 | test.cpp:208:7:208:7 | y | test.cpp:190:10:190:13 | call to rand | test.cpp:208:7:208:7 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:190:10:190:13 | call to rand | Uncontrolled value |
 | test.cpp:219:8:219:8 | x | test.cpp:215:11:215:14 | call to rand | test.cpp:219:8:219:8 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:215:11:215:14 | call to rand | Uncontrolled value |
-| test.cpp:227:8:227:8 | x | test.cpp:223:20:223:23 | call to rand | test.cpp:227:8:227:8 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:223:20:223:23 | call to rand | Uncontrolled value |
-| test.cpp:227:8:227:8 | x | test.cpp:223:20:223:25 | (unsigned int)... | test.cpp:227:8:227:8 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:223:20:223:23 | call to rand | Uncontrolled value |

cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.cpp

@@ -224,6 +224,6 @@ void test_mod_limit()
 		unsigned int y = 100;
 		unsigned int z;
 
-		z = (x + y) % 1000; // DUBIOUS (this could overflow but the result is controlled) [REPORTED]
+		z = (x + y) % 1000; // DUBIOUS (this could overflow but the result is controlled)
 	}
 }