ArithmeticUncontrolled: float exprs are barriers.

This leads to two regressions, but this should be OK if we decide to drop float support from SimpleRangeAnalysis Keeping high query precision to keep it in code scanning.
github · Jul 5, 2024 · 6589fc5 · 6589fc5
1 parent 252bb9e
commit 6589fc5
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 37 deletions.
diff --git a/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql b/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
@@ -117,6 +117,8 @@ module UncontrolledArithConfig implements DataFlow::ConfigSig {
         op instanceof ComplementExpr
       ).getAnOperand*()
     or
+    node.asExpr().getUnderlyingType() instanceof FloatingPointType
+    or
     // block unintended flow to pointers
     node.asExpr().getUnspecifiedType() instanceof PointerType
   }

diff --git a/...-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected b/...-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected
@@ -11,11 +11,6 @@ edges
 | test.c:81:13:81:29 | ... ^ ... | test.c:83:9:83:9 | r | provenance |  |
 | test.c:81:14:81:17 | call to rand | test.c:81:13:81:29 | ... ^ ... | provenance |  |
 | test.c:81:23:81:26 | call to rand | test.c:81:13:81:29 | ... ^ ... | provenance |  |
-| test.c:104:13:104:16 | call to rand | test.c:104:13:104:16 | call to rand | provenance |  |
-| test.c:104:13:104:16 | call to rand | test.c:105:5:105:42 | ... = ... | provenance |  |
-| test.c:105:5:105:42 | ... = ... | test.c:106:13:106:13 | r | provenance |  |
-| test.c:106:13:106:13 | r | test.c:110:18:110:18 | r | provenance |  |
-| test.c:110:18:110:18 | r | test.c:111:3:111:3 | r | provenance |  |
 | test.c:125:13:125:16 | call to rand | test.c:125:13:125:16 | call to rand | provenance |  |
 | test.c:125:13:125:16 | call to rand | test.c:127:9:127:9 | r | provenance |  |
 | test.c:131:13:131:16 | call to rand | test.c:131:13:131:16 | call to rand | provenance |  |
@@ -47,9 +42,6 @@ edges
 | test.cpp:151:10:151:13 | call to rand | test.cpp:151:10:151:13 | call to rand | provenance |  |
 | test.cpp:151:10:151:13 | call to rand | test.cpp:153:10:153:15 | ... - ... | provenance |  |
 | test.cpp:153:10:153:15 | ... - ... | test.cpp:154:10:154:10 | b | provenance |  |
-| test.cpp:169:11:169:14 | call to rand | test.cpp:169:11:169:14 | call to rand | provenance |  |
-| test.cpp:169:11:169:14 | call to rand | test.cpp:170:13:170:13 | x | provenance |  |
-| test.cpp:170:13:170:13 | x | test.cpp:171:11:171:16 | y | provenance |  |
 | test.cpp:189:10:189:13 | call to rand | test.cpp:189:10:189:13 | call to rand | provenance |  |
 | test.cpp:189:10:189:13 | call to rand | test.cpp:195:3:195:11 | ... = ... | provenance |  |
 | test.cpp:189:10:189:13 | call to rand | test.cpp:198:3:198:11 | ... = ... | provenance |  |
@@ -82,12 +74,6 @@ nodes
 | test.c:81:14:81:17 | call to rand | semmle.label | call to rand |
 | test.c:81:23:81:26 | call to rand | semmle.label | call to rand |
 | test.c:83:9:83:9 | r | semmle.label | r |
-| test.c:104:13:104:16 | call to rand | semmle.label | call to rand |
-| test.c:104:13:104:16 | call to rand | semmle.label | call to rand |
-| test.c:105:5:105:42 | ... = ... | semmle.label | ... = ... |
-| test.c:106:13:106:13 | r | semmle.label | r |
-| test.c:110:18:110:18 | r | semmle.label | r |
-| test.c:111:3:111:3 | r | semmle.label | r |
 | test.c:125:13:125:16 | call to rand | semmle.label | call to rand |
 | test.c:125:13:125:16 | call to rand | semmle.label | call to rand |
 | test.c:127:9:127:9 | r | semmle.label | r |
@@ -130,10 +116,6 @@ nodes
 | test.cpp:151:10:151:13 | call to rand | semmle.label | call to rand |
 | test.cpp:153:10:153:15 | ... - ... | semmle.label | ... - ... |
 | test.cpp:154:10:154:10 | b | semmle.label | b |
-| test.cpp:169:11:169:14 | call to rand | semmle.label | call to rand |
-| test.cpp:169:11:169:14 | call to rand | semmle.label | call to rand |
-| test.cpp:170:13:170:13 | x | semmle.label | x |
-| test.cpp:171:11:171:16 | y | semmle.label | y |
 | test.cpp:189:10:189:13 | call to rand | semmle.label | call to rand |
 | test.cpp:189:10:189:13 | call to rand | semmle.label | call to rand |
 | test.cpp:190:10:190:13 | call to rand | semmle.label | call to rand |
@@ -160,7 +142,6 @@ subpaths
 | test.c:77:9:77:9 | r | test.c:75:13:75:19 | call to rand | test.c:77:9:77:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:75:13:75:19 | call to rand | uncontrolled value |
 | test.c:83:9:83:9 | r | test.c:81:14:81:17 | call to rand | test.c:83:9:83:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:81:14:81:17 | call to rand | uncontrolled value |
 | test.c:83:9:83:9 | r | test.c:81:23:81:26 | call to rand | test.c:83:9:83:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:81:23:81:26 | call to rand | uncontrolled value |
-| test.c:111:3:111:3 | r | test.c:104:13:104:16 | call to rand | test.c:111:3:111:3 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:104:13:104:16 | call to rand | uncontrolled value |
 | test.c:127:9:127:9 | r | test.c:125:13:125:16 | call to rand | test.c:127:9:127:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:125:13:125:16 | call to rand | uncontrolled value |
 | test.c:133:5:133:5 | r | test.c:131:13:131:16 | call to rand | test.c:133:5:133:5 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:131:13:131:16 | call to rand | uncontrolled value |
 | test.c:139:10:139:10 | r | test.c:137:13:137:16 | call to rand | test.c:139:10:139:10 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:137:13:137:16 | call to rand | uncontrolled value |
@@ -172,7 +153,6 @@ subpaths
 | test.cpp:102:10:102:10 | x | test.cpp:98:10:98:13 | call to rand | test.cpp:102:10:102:10 | x | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:98:10:98:13 | call to rand | uncontrolled value |
 | test.cpp:146:9:146:9 | y | test.cpp:137:10:137:13 | call to rand | test.cpp:146:9:146:9 | y | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:137:10:137:13 | call to rand | uncontrolled value |
 | test.cpp:154:10:154:10 | b | test.cpp:151:10:151:13 | call to rand | test.cpp:154:10:154:10 | b | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:151:10:151:13 | call to rand | uncontrolled value |
-| test.cpp:171:11:171:16 | y | test.cpp:169:11:169:14 | call to rand | test.cpp:171:11:171:16 | y | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:169:11:169:14 | call to rand | uncontrolled value |
 | test.cpp:196:7:196:7 | x | test.cpp:189:10:189:13 | call to rand | test.cpp:196:7:196:7 | x | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:189:10:189:13 | call to rand | uncontrolled value |
 | test.cpp:198:7:198:7 | x | test.cpp:189:10:189:13 | call to rand | test.cpp:198:7:198:7 | x | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:189:10:189:13 | call to rand | uncontrolled value |
 | test.cpp:199:7:199:7 | x | test.cpp:189:10:189:13 | call to rand | test.cpp:199:7:199:7 | x | This arithmetic expression depends on an $@, potentially causing an overflow. | test.cpp:189:10:189:13 | call to rand | uncontrolled value |

diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.c b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.c
@@ -16,8 +16,8 @@ void randomTester() {
   int i;
   for (i = 0; i < 1000; i++) {
     int r = rand();
-    
-    // BAD: The return from rand() is unbounded 
+
+    // BAD: The return from rand() is unbounded
     trySlice(r, r+100);
   }
 
@@ -44,24 +44,24 @@ void randomTester() {
     int r = rand();
     r += 100; // BAD
   }
-  
+
   {
     int r = rand() / 10;
     r += 100; // GOOD
   }
-  
+
   {
     int r = rand();
     r = r / 10;
     r += 100; // GOOD
   }
-  
+
   {
     int r = rand();
     r /= 10;
     r += 100; // GOOD
   }
-  
+
   {
     int r = rand() & 0xFF;
     r += 100; // GOOD
@@ -108,7 +108,7 @@ void randomTester() {
 }
 
 void add_100(int r) {
-  r += 100; // GOOD [FALSE POSITIVE]
+  r += 100; // GOOD
 }
 
 void randomTester2(int bound, int min, int max) {
@@ -123,13 +123,13 @@ void randomTester2(int bound, int min, int max) {
 void moreTests() {
   {
     int r = rand();
-    
+
     r = r * 100; // BAD
   }
 
   {
     int r = rand();
-    
+
     r *= 100; // BAD
   }
 
@@ -141,19 +141,19 @@ void moreTests() {
 
   {
     int r = rand();
-    
+
     r <<= 8; // BAD [NOT DETECTED]
   }
 
   {
     int r = rand();
-    
+
     r = r - 100; // GOOD
   }
 
   {
     unsigned int r = rand();
-    
+
     r = r - 100; // BAD
   }
 }
@@ -164,4 +164,4 @@ void guarded_test(unsigned p) {
     return;
   }
   unsigned z = data - p; // GOOD
-}
+}
diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.cpp
@@ -128,7 +128,7 @@ int test_conditional_assignment_2()
 	{
 		y = x;
 	}
-	
+
 	return y * 10; // GOOD (as y <= 100)
 }
 
@@ -142,7 +142,7 @@ int test_conditional_assignment_3()
 	{
 		y = x;
 	}
-	
+
 	return y * c; // GOOD (as y <= 100) [FALSE POSITIVE]
 }
 
@@ -168,7 +168,7 @@ void test_float()
 	{
 		int x = rand();
 		float y = x; // GOOD
-		int z = (int)y * 5; // BAD
+		int z = (int)y * 5; // BAD [NOT DETECTED]
 	}
 
 	{