diff --git a/cpp/ql/src/experimental/Likely Bugs/OverrunWriteProductFlow.ql b/cpp/ql/src/experimental/Likely Bugs/OverrunWriteProductFlow.ql index 7c982b09151..128d766d3b9 100644 --- a/cpp/ql/src/experimental/Likely Bugs/OverrunWriteProductFlow.ql +++ b/cpp/ql/src/experimental/Likely Bugs/OverrunWriteProductFlow.ql @@ -134,16 +134,34 @@ module StringSizeConfig implements ProductFlow::StateConfigSig { module StringSizeFlow = ProductFlow::GlobalWithState; +int getOverflow( + DataFlow::Node source1, DataFlow::Node source2, DataFlow::Node sink1, DataFlow::Node sink2, + CallInstruction c, Expr buffer +) { + result > 0 and + exists( + StringSizeFlow::PathNode1 pathSource1, StringSizeFlow::PathNode2 pathSource2, + StringSizeFlow::PathNode1 pathSink1, StringSizeFlow::PathNode2 pathSink2 + | + StringSizeFlow::flowPath(pathSource1, pathSource2, pathSink1, pathSink2) and + source1 = pathSource1.getNode() and + source2 = pathSource2.getNode() and + sink1 = pathSink1.getNode() and + sink2 = pathSink2.getNode() and + isSinkPairImpl(c, sink1, sink2, result + pathSink2.getState(), buffer) + ) +} + from StringSizeFlow::PathNode1 source1, StringSizeFlow::PathNode2 source2, - StringSizeFlow::PathNode1 sink1, StringSizeFlow::PathNode2 sink2, int overflow, int sinkState, - CallInstruction c, DataFlow::Node sourceNode, Expr buffer, string element + StringSizeFlow::PathNode1 sink1, StringSizeFlow::PathNode2 sink2, int overflow, CallInstruction c, + Expr buffer, string element where StringSizeFlow::flowPath(source1, source2, sink1, sink2) and - sinkState = sink2.getState() and - isSinkPairImpl(c, sink1.getNode(), sink2.getNode(), overflow + sinkState, buffer) and - overflow > 0 and - sourceNode = source1.getNode() and + overflow = + max(getOverflow(source1.getNode(), source2.getNode(), sink1.getNode(), sink2.getNode(), c, + buffer) + ) and if overflow = 1 then element = " element." else element = " elements." select c.getUnconvertedResultExpression(), source1, sink1, "This write may overflow $@ by " + overflow + element, buffer, buffer.toString() diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected index 1abbeb2b57f..bca05e2a4ef 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected @@ -212,15 +212,16 @@ edges | test.cpp:214:24:214:24 | p | test.cpp:216:10:216:10 | p | | test.cpp:220:43:220:48 | call to malloc | test.cpp:222:15:222:20 | buffer | | test.cpp:222:15:222:20 | buffer | test.cpp:214:24:214:24 | p | -| test.cpp:225:40:225:45 | buffer | test.cpp:226:5:226:26 | ... = ... | -| test.cpp:226:5:226:26 | ... = ... | test.cpp:226:12:226:17 | p_str indirection [post update] [string] | -| test.cpp:231:27:231:32 | call to malloc | test.cpp:232:22:232:27 | buffer | -| test.cpp:232:16:232:19 | set_string output argument [string] | test.cpp:233:12:233:14 | str indirection [string] | -| test.cpp:232:22:232:27 | buffer | test.cpp:225:40:225:45 | buffer | -| test.cpp:232:22:232:27 | buffer | test.cpp:232:16:232:19 | set_string output argument [string] | -| test.cpp:233:12:233:14 | str indirection [string] | test.cpp:233:12:233:21 | string | -| test.cpp:233:12:233:14 | str indirection [string] | test.cpp:233:16:233:21 | string indirection | -| test.cpp:233:16:233:21 | string indirection | test.cpp:233:12:233:21 | string | +| test.cpp:228:43:228:48 | call to malloc | test.cpp:232:10:232:15 | buffer | +| test.cpp:235:40:235:45 | buffer | test.cpp:236:5:236:26 | ... = ... | +| test.cpp:236:5:236:26 | ... = ... | test.cpp:236:12:236:17 | p_str indirection [post update] [string] | +| test.cpp:241:27:241:32 | call to malloc | test.cpp:242:22:242:27 | buffer | +| test.cpp:242:16:242:19 | set_string output argument [string] | test.cpp:243:12:243:14 | str indirection [string] | +| test.cpp:242:22:242:27 | buffer | test.cpp:235:40:235:45 | buffer | +| test.cpp:242:22:242:27 | buffer | test.cpp:242:16:242:19 | set_string output argument [string] | +| test.cpp:243:12:243:14 | str indirection [string] | test.cpp:243:12:243:21 | string | +| 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 | 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 | ... = ... | @@ -390,17 +391,19 @@ nodes | test.cpp:216:10:216:10 | p | semmle.label | p | | test.cpp:220:43:220:48 | call to malloc | semmle.label | call to malloc | | test.cpp:222:15:222:20 | buffer | semmle.label | buffer | -| test.cpp:225:40:225:45 | buffer | semmle.label | buffer | -| test.cpp:226:5:226:26 | ... = ... | semmle.label | ... = ... | -| test.cpp:226:12:226:17 | p_str indirection [post update] [string] | semmle.label | p_str indirection [post update] [string] | -| test.cpp:231:27:231:32 | call to malloc | semmle.label | call to malloc | -| test.cpp:232:16:232:19 | set_string output argument [string] | semmle.label | set_string output argument [string] | -| test.cpp:232:22:232:27 | buffer | semmle.label | buffer | -| test.cpp:233:12:233:14 | str indirection [string] | semmle.label | str indirection [string] | -| test.cpp:233:12:233:21 | string | semmle.label | string | -| test.cpp:233:16:233:21 | string indirection | semmle.label | string indirection | +| test.cpp:228:43:228:48 | call to malloc | semmle.label | call to malloc | +| test.cpp:232:10:232:15 | buffer | semmle.label | buffer | +| test.cpp:235:40:235:45 | buffer | semmle.label | buffer | +| test.cpp:236:5:236:26 | ... = ... | semmle.label | ... = ... | +| test.cpp:236:12:236:17 | p_str indirection [post update] [string] | semmle.label | p_str indirection [post update] [string] | +| test.cpp:241:27:241:32 | call to malloc | semmle.label | call to malloc | +| test.cpp:242:16:242:19 | set_string output argument [string] | semmle.label | set_string output argument [string] | +| test.cpp:242:22:242:27 | buffer | semmle.label | buffer | +| test.cpp:243:12:243:14 | str indirection [string] | semmle.label | str indirection [string] | +| test.cpp:243:12:243:21 | string | semmle.label | string | +| test.cpp:243:16:243:21 | string indirection | semmle.label | string indirection | subpaths -| test.cpp:232:22:232:27 | buffer | test.cpp:225:40:225:45 | buffer | test.cpp:226:12:226:17 | p_str indirection [post update] [string] | test.cpp:232:16:232:19 | set_string output argument [string] | +| 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 | test.cpp:42:5:42:11 | call to strncpy | test.cpp:18:19:18:24 | call to malloc | test.cpp:42:18:42:23 | string | This write may overflow $@ by 1 element. | test.cpp:42:18:42:23 | string | string | | test.cpp:72:9:72:15 | call to strncpy | test.cpp:18:19:18:24 | call to malloc | test.cpp:72:22:72:27 | string | This write may overflow $@ by 1 element. | test.cpp:72:22:72:27 | string | string | @@ -417,4 +420,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:233:5:233:10 | call to memset | test.cpp:231:27:231:32 | call to malloc | test.cpp:233:12:233:21 | string | This write may overflow $@ by 1 element. | test.cpp:233:16:233:21 | 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 | diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/test.cpp index 8f4457fd2d4..fe54fc86b2d 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/test.cpp @@ -222,6 +222,16 @@ void test_missing_call_context(unsigned char *unrelated_buffer, unsigned size) { call_memset(buffer, size); } +bool unknown(); + +void repeated_alerts(unsigned size, unsigned offset) { + unsigned char* buffer = (unsigned char*)malloc(size); + while(unknown()) { + ++size; + } + memset(buffer, 0, size); // BAD +} + void set_string(string_t* p_str, char* buffer) { p_str->string = buffer; } @@ -232,3 +242,4 @@ void test_flow_through_setter(unsigned size) { set_string(&str, buffer); memset(str.string, 0, size + 1); // BAD } +