diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll b/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll index 656e54e68a8..f2276b66dde 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll @@ -80,7 +80,7 @@ private predicate localFlowToExprStep(DataFlow::Node n1, DataFlow::Node n2) { /** Holds if `n2 + delta` may be equal to `n1`. */ private predicate localFlowStepToExpr(Expr e1, Expr e2) { - getBufferSize0(e1) and + getBufferSizeCand0(e1) and exists(DataFlow::Node n1, DataFlow::Node mid, DataFlow::Node n2 | n1.asExpr() = e1 and localFlowToExprStep*(n1, mid) and @@ -95,7 +95,7 @@ private predicate localFlowStepToExpr(Expr e1, Expr e2) { * expression. */ private predicate step(Expr e1, Expr e2, int delta) { - getBufferSize0(e1) and + getBufferSizeCand0(e1) and exists(Variable bufferVar, Class parentClass, VariableAccess parentPtr, int bufferSize | e1 = parentPtr | @@ -117,27 +117,37 @@ private predicate step(Expr e1, Expr e2, int delta) { } pragma[nomagic] -private predicate getBufferSize0(Expr e) { +private predicate getBufferSizeCand0(Expr e) { exists(isSource(e, _)) or exists(Expr e0 | - getBufferSize0(e0) and + getBufferSizeCand0(e0) and step(e0, e, _) ) } +/** + * Get the size in bytes of the buffer pointed to by an expression (if this can be determined). + * + * NOTE: There can be multiple `(result, why)` for a given `bufferExpr`. + */ +private int getBufferSizeCand(Expr bufferExpr, Element why) { + getBufferSizeCand0(bufferExpr) and + ( + result = isSource(bufferExpr, why) + or + exists(Expr e0, int delta, int size | + size = getBufferSizeCand(e0, why) and + step(e0, bufferExpr, delta) and + result = size + delta + ) + ) +} + /** * Get the size in bytes of the buffer pointed to by an expression (if this can be determined). */ int getBufferSize(Expr bufferExpr, Element why) { - getBufferSize0(bufferExpr) and - ( - result = isSource(bufferExpr, why) - or - exists(Expr e0, int delta, int size | - size = getBufferSize(e0, why) and - delta = unique(int cand | step(e0, bufferExpr, cand) | cand) and - result = size + delta - ) - ) + result = max( | | getBufferSizeCand(bufferExpr, _)) and + result = getBufferSizeCand(bufferExpr, why) } diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-119/SAMATE/VeryLikelyOverrunWrite.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-119/SAMATE/VeryLikelyOverrunWrite.expected index a8e1f1f8f3a..e69de29bb2d 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-119/SAMATE/VeryLikelyOverrunWrite.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-119/SAMATE/VeryLikelyOverrunWrite.expected @@ -1 +0,0 @@ -| tests.cpp:668:9:668:14 | call to strcpy | This 'call to strcpy' operation requires 11 bytes but the destination is only 10 bytes. | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowBuffer.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowBuffer.expected index f0dc2bfc3ec..252e581d2d5 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowBuffer.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowBuffer.expected @@ -48,7 +48,6 @@ | tests.cpp:495:2:495:7 | access to array | This array indexing operation accesses a negative index -1 on the $@. | tests.cpp:474:21:474:26 | call to malloc | array | | tests.cpp:519:3:519:8 | call to memset | This 'memset' operation accesses 20 bytes but the $@ is only 10 bytes. | tests.cpp:502:15:502:20 | call to malloc | destination buffer | | tests.cpp:519:3:519:8 | call to memset | This 'memset' operation accesses 20 bytes but the $@ is only 10 bytes. | tests.cpp:510:16:510:21 | call to malloc | destination buffer | -| tests.cpp:520:3:520:8 | call to memset | This 'memset' operation accesses 20 bytes but the $@ is only 10 bytes. | tests.cpp:503:15:503:20 | call to malloc | destination buffer | | tests.cpp:541:6:541:10 | call to fread | This 'fread' operation may access 101 bytes but the $@ is only 100 bytes. | tests.cpp:532:7:532:16 | charBuffer | destination buffer | | tests.cpp:546:6:546:10 | call to fread | This 'fread' operation may access 400 bytes but the $@ is only 100 bytes. | tests.cpp:532:7:532:16 | charBuffer | destination buffer | | tests.cpp:569:6:569:15 | access to array | This array indexing operation accesses a negative index -1 on the $@. | tests.cpp:565:7:565:12 | buffer | array | @@ -57,9 +56,6 @@ | tests.cpp:586:6:586:12 | access to array | This array indexing operation accesses a negative index -1 on the $@. | tests.cpp:565:7:565:12 | buffer | array | | tests_restrict.c:12:2:12:7 | call to memcpy | This 'memcpy' operation accesses 2 bytes but the $@ is only 1 byte. | tests_restrict.c:7:6:7:13 | smallbuf | source buffer | | unions.cpp:26:2:26:7 | call to memset | This 'memset' operation accesses 200 bytes but the $@ is only 100 bytes. | unions.cpp:21:10:21:11 | mu | destination buffer | -| unions.cpp:27:2:27:7 | call to memset | This 'memset' operation accesses 100 bytes but the $@ is only 10 bytes. | unions.cpp:15:7:15:11 | small | destination buffer | -| unions.cpp:29:2:29:7 | call to memset | This 'memset' operation accesses 100 bytes but the $@ is only 10 bytes. | unions.cpp:15:7:15:11 | small | destination buffer | -| unions.cpp:30:2:30:7 | call to memset | This 'memset' operation accesses 200 bytes but the $@ is only 10 bytes. | unions.cpp:15:7:15:11 | small | destination buffer | | unions.cpp:30:2:30:7 | call to memset | This 'memset' operation accesses 200 bytes but the $@ is only 100 bytes. | unions.cpp:15:7:15:11 | small | destination buffer | | unions.cpp:34:2:34:7 | call to memset | This 'memset' operation accesses 200 bytes but the $@ is only 100 bytes. | unions.cpp:16:7:16:11 | large | destination buffer | | var_size_struct.cpp:71:3:71:8 | call to memset | This 'memset' operation accesses 1025 bytes but the $@ is only 1024 bytes. | var_size_struct.cpp:67:35:67:40 | call to malloc | destination buffer | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/tests.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/tests.cpp index f99003e666d..49c02b9f199 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/tests.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/tests.cpp @@ -517,7 +517,7 @@ void test19(bool b) if (b) { memset(p1, 0, 20); // BAD - memset(p2, 0, 20); // GOOD [FALSE POSITIVE] + memset(p2, 0, 20); // GOOD memset(p3, 0, 20); // GOOD } }