C++: exclude end pointers in iterator-style loops

Author: rdmarsh2

cpp/ql/src/experimental/Likely Bugs/ArrayAccessProductFlow.ql

@@ -21,7 +21,6 @@ class RangeAnalysisIRConfig extends IRConfiguration {
   }
 }
 
-
 predicate bounded(Instruction i, Bound b, int delta, boolean upper) {
   // TODO: reason
   semBounded(getSemanticExpr(i), b, delta, upper, _)
@@ -31,9 +30,9 @@ class ArraySizeConfiguration extends ProductFlow::Configuration {
   ArraySizeConfiguration() { this = "ArraySizeConfiguration" }
 
   override predicate isSourcePair(DataFlow::Node source1, DataFlow::Node source2) {
-    exists(GVN sizeGVN |
-      source1.asConvertedExpr().(AllocationExpr).getSizeExpr() = sizeGVN.getAnExpr() and
-      source2.asConvertedExpr() = sizeGVN.getAnExpr()
+    exists(GVN sizeGvn |
+      source1.asConvertedExpr().(AllocationExpr).getSizeExpr() = sizeGvn.getAnExpr() and
+      source2.asConvertedExpr() = sizeGvn.getAnExpr()
     )
   }
 
@@ -43,7 +42,16 @@ class ArraySizeConfiguration extends ProductFlow::Configuration {
       pai.getLeft() = sink1.asInstruction() and
       bounded(index, b, delta, true) and
       sink2.asInstruction() = b.getInstruction() and
-      delta >= 0
+      (
+        delta = 0 and
+        exists(DataFlow::Node paiNode, DataFlow::Node derefNode |
+          DataFlow::localFlow(paiNode, derefNode) and
+          paiNode.asInstruction() = pai and
+          derefNode.asOperand() instanceof AddressOperand
+        )
+        or
+        delta >= 1
+      )
     )
   }
 }

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/array-access/ArrayAccessProductFlow.expected

@@ -21,3 +21,23 @@
 | test.cpp:70:14:70:19 | call to malloc | test.cpp:69:17:69:20 | size | test.cpp:83:14:83:14 | p | test.cpp:82:31:82:34 | size |
 | test.cpp:70:14:70:19 | call to malloc | test.cpp:69:17:69:20 | size | test.cpp:93:14:93:14 | p | test.cpp:88:30:88:33 | size |
 | test.cpp:70:14:70:19 | call to malloc | test.cpp:69:17:69:20 | size | test.cpp:93:14:93:14 | p | test.cpp:92:31:92:34 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:103:24:103:27 | size | test.cpp:104:29:104:31 | arr | test.cpp:103:24:103:27 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:103:24:103:27 | size | test.cpp:104:29:104:31 | arr | test.cpp:103:24:103:27 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:103:24:103:27 | size | test.cpp:104:29:104:31 | arr | test.cpp:104:35:104:38 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:103:24:103:27 | size | test.cpp:104:29:104:31 | arr | test.cpp:104:35:104:38 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:103:24:103:27 | size | test.cpp:104:29:104:31 | arr | test.cpp:108:36:108:39 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:103:24:103:27 | size | test.cpp:108:30:108:32 | arr | test.cpp:103:24:103:27 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:103:24:103:27 | size | test.cpp:108:30:108:32 | arr | test.cpp:103:24:103:27 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:103:24:103:27 | size | test.cpp:108:30:108:32 | arr | test.cpp:104:35:104:38 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:103:24:103:27 | size | test.cpp:108:30:108:32 | arr | test.cpp:104:35:104:38 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:103:24:103:27 | size | test.cpp:108:30:108:32 | arr | test.cpp:108:36:108:39 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:104:35:104:38 | size | test.cpp:104:29:104:31 | arr | test.cpp:104:35:104:38 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:104:35:104:38 | size | test.cpp:104:29:104:31 | arr | test.cpp:104:35:104:38 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:104:35:104:38 | size | test.cpp:104:29:104:31 | arr | test.cpp:108:36:108:39 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:104:35:104:38 | size | test.cpp:108:30:108:32 | arr | test.cpp:104:35:104:38 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:104:35:104:38 | size | test.cpp:108:30:108:32 | arr | test.cpp:104:35:104:38 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:104:35:104:38 | size | test.cpp:108:30:108:32 | arr | test.cpp:108:36:108:39 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:108:36:108:39 | size | test.cpp:104:29:104:31 | arr | test.cpp:108:36:108:39 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:108:36:108:39 | size | test.cpp:104:29:104:31 | arr | test.cpp:108:36:108:39 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:108:36:108:39 | size | test.cpp:108:30:108:32 | arr | test.cpp:108:36:108:39 | size |
+| test.cpp:103:17:103:22 | call to malloc | test.cpp:108:36:108:39 | size | test.cpp:108:30:108:32 | arr | test.cpp:108:36:108:39 | size |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/array-access/test.cpp

@@ -3,11 +3,11 @@ char *malloc(int size);
 void test1(int size) {
     char *arr = malloc(size);
     for (int i = 0; i < size; i++) {
-        arr[i] = 0;
+        arr[i] = 0; // GOOD
     }
 
     for (int i = 0; i <= size; i++) {
-        arr[i] = i;
+        arr[i] = i; // BAD
     }
 }
 
@@ -32,7 +32,7 @@ void test2(int size) {
     }
 
     for (int i = 0; i <= arr.size; i++) {
-        arr.p[i] = i; // BAD
+        arr.p[i] = i; // BAD [NOT DETECTED]
     }
 }
 
@@ -42,7 +42,7 @@ void test3_callee(array_t arr) {
     }
 
     for (int i = 0; i <= arr.size; i++) {
-        arr.p[i] = i; // BAD
+        arr.p[i] = i; // BAD [NOT DETECTED]
     }
 }
 
@@ -96,4 +96,15 @@ void test6_callee(array_t *arr) {
 
 void test6(int size) {
     test6_callee(mk_array_p(size));
-}
+}
+
+void test7(int size) {
+    char *arr = malloc(size);
+    for (char *p = arr; p < arr + size; p++) {
+        *p = 0; // GOOD
+    }
+
+    for (char *p = arr; p <= arr + size; p++) {
+        *p = 0; // BAD [NOT DETECTED]
+    }
+}