Merge pull request github#13149 from MathiasVP/barrier-out-on-phi-back-edges

C++: Block flow through back-edges in `cpp/overrun-write`

Author: jketema

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

@@ -111,6 +111,10 @@ module StringSizeConfig implements ProductFlow::StateConfigSig {
 
   predicate isBarrier2(DataFlow::Node node, FlowState2 state) { none() }
 
+  predicate isBarrierOut2(DataFlow::Node node) {
+    node = any(DataFlow::SsaPhiNode phi).getAnInput(true)
+  }
+
   predicate isAdditionalFlowStep1(
     DataFlow::Node node1, FlowState1 state1, DataFlow::Node node2, FlowState1 state2
   ) {

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected

@@ -223,6 +223,7 @@ edges
 | test.cpp:243:12:243:14 | str indirection [string] | test.cpp:243:16:243:21 | string indirection |
 | test.cpp:243:16:243:21 | string indirection | test.cpp:243:12:243:21 | string |
 | test.cpp:249:20:249:27 | call to my_alloc | test.cpp:250:12:250:12 | p |
+| test.cpp:256:17:256:22 | call to malloc | test.cpp:257:12:257:12 | p |
 nodes
 | test.cpp:16:11:16:21 | mk_string_t indirection [string] | semmle.label | mk_string_t indirection [string] |
 | test.cpp:18:5:18:30 | ... = ... | semmle.label | ... = ... |
@@ -405,6 +406,8 @@ nodes
 | test.cpp:243:16:243:21 | string indirection | semmle.label | string indirection |
 | test.cpp:249:20:249:27 | call to my_alloc | semmle.label | call to my_alloc |
 | test.cpp:250:12:250:12 | p | semmle.label | p |
+| test.cpp:256:17:256:22 | call to malloc | semmle.label | call to malloc |
+| test.cpp:257:12:257:12 | p | semmle.label | p |
 subpaths
 | test.cpp:242:22:242:27 | buffer | test.cpp:235:40:235:45 | buffer | test.cpp:236:12:236:17 | p_str indirection [post update] [string] | test.cpp:242:16:242:19 | set_string output argument [string] |
 #select
@@ -423,6 +426,5 @@ subpaths
 | test.cpp:199:9:199:15 | call to strncpy | test.cpp:147:19:147:24 | call to malloc | test.cpp:199:22:199:27 | string | This write may overflow $@ by 2 elements. | test.cpp:199:22:199:27 | string | string |
 | test.cpp:203:9:203:15 | call to strncpy | test.cpp:147:19:147:24 | call to malloc | test.cpp:203:22:203:27 | string | This write may overflow $@ by 2 elements. | test.cpp:203:22:203:27 | string | string |
 | test.cpp:207:9:207:15 | call to strncpy | test.cpp:147:19:147:24 | call to malloc | test.cpp:207:22:207:27 | string | This write may overflow $@ by 3 elements. | test.cpp:207:22:207:27 | string | string |
-| test.cpp:232:3:232:8 | call to memset | test.cpp:228:43:228:48 | call to malloc | test.cpp:232:10:232:15 | buffer | This write may overflow $@ by 32 elements. | test.cpp:232:10:232:15 | buffer | buffer |
 | test.cpp:243:5:243:10 | call to memset | test.cpp:241:27:241:32 | call to malloc | test.cpp:243:12:243:21 | string | This write may overflow $@ by 1 element. | test.cpp:243:16:243:21 | string | string |
 | test.cpp:250:5:250:10 | call to memset | test.cpp:249:20:249:27 | call to my_alloc | test.cpp:250:12:250:12 | p | This write may overflow $@ by 1 element. | test.cpp:250:12:250:12 | p | p |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/test.cpp

@@ -229,7 +229,7 @@ void repeated_alerts(unsigned size, unsigned offset) {
   while(unknown()) {
     ++size;
   }
-  memset(buffer, 0, size); // BAD
+  memset(buffer, 0, size); // BAD [NOT DETECTED]
 }
 
 void set_string(string_t* p_str, char* buffer) {
@@ -248,4 +248,12 @@ void* my_alloc(unsigned size);
 void foo(unsigned size) {
     int* p = (int*)my_alloc(size); // BAD
     memset(p, 0, size + 1);
+}
+
+void test6(unsigned long n, char *p) {
+  while (unknown()) {
+    n++;
+    p = (char *)malloc(n);
+    memset(p, 0, n); // GOOD
+  }
 }