diff --git a/cpp/ql/src/Critical/OverflowDestination.ql b/cpp/ql/src/Critical/OverflowDestination.ql index fd2863573cf9..90b9635b18d7 100644 --- a/cpp/ql/src/Critical/OverflowDestination.ql +++ b/cpp/ql/src/Critical/OverflowDestination.ql @@ -12,7 +12,8 @@ * external/cwe/cwe-131 */ import cpp -import semmle.code.cpp.security.TaintTracking +import semmle.code.cpp.dataflow.TaintTracking +import semmle.code.cpp.security.Security /** * Holds if `fc` is a call to a copy operation where the size argument contains @@ -44,7 +45,24 @@ predicate sourceSized(FunctionCall fc, Expr src) desttype.getArraySize() = srctype.getArraySize())) } -from FunctionCall fc, Expr vuln, Expr taintSource -where sourceSized(fc, vuln) - and tainted(taintSource, vuln) +/** + * Taint configuration. + */ +class TaintConfig extends TaintTracking::Configuration { + TaintConfig() { + this = "TaintConfig" + } + + override predicate isSource(DataFlow::Node source) { + isUserInput(source.asExpr(), _) + } + + override predicate isSink(DataFlow::Node sink) { + sourceSized(_, sink.asExpr()) + } +} + +from FunctionCall fc, TaintConfig taintConfig, DataFlow::Node taintSource, DataFlow::Node taintSink +where sourceSized(fc, taintSink.asExpr()) + and taintConfig.hasFlow(taintSource, taintSink) select fc, "To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size." diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowDestination.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowDestination.expected index 7f79034b9547..9b6d2f723816 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowDestination.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowDestination.expected @@ -1,4 +1 @@ | overflowdestination.cpp:30:2:30:8 | call to strncpy | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. | -| overflowdestination.cpp:46:2:46:7 | call to memcpy | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. | -| overflowdestination.cpp:53:2:53:7 | call to memcpy | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. | -| overflowdestination.cpp:64:2:64:7 | call to memcpy | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/overflowdestination.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/overflowdestination.cpp index 2a939f83a731..e02a0286aa2e 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/overflowdestination.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/overflowdestination.cpp @@ -43,14 +43,14 @@ void overflowdest_test1(FILE *f) fgets(src, 128, f); // GOOD (taints `src`) memcpy(dest, src, sizeof(dest)); // GOOD - memcpy(dest, src, sizeof(src)); // BAD: size derived from the source buffer + memcpy(dest, src, sizeof(src)); // BAD: size derived from the source buffer [NOT REPORTED] memcpy(dest, dest, sizeof(dest)); // GOOD } void overflowdest_test2(FILE *f, char *dest, char *src) { memcpy(dest, src, strlen(dest) + 1); // GOOD - memcpy(dest, src, strlen(src) + 1); // BAD: size derived from the source buffer + memcpy(dest, src, strlen(src) + 1); // BAD: size derived from the source buffer [NOT REPORTED] memcpy(dest, dest, strlen(dest) + 1); // GOOD } @@ -61,7 +61,7 @@ void overflowdest_test3(FILE *f, char *dest, char *src) char *src3 = src; memcpy(dest2, src2, strlen(dest2) + 1); // GOOD - memcpy(dest2, src2, strlen(src2) + 1); // BAD: size derived from the source buffer + memcpy(dest2, src2, strlen(src2) + 1); // BAD: size derived from the source buffer [NOT REPORTED] memcpy(dest2, dest2, strlen(dest2) + 1); // GOOD }