Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions cpp/ql/src/Critical/OverflowDestination.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."
Original file line number Diff line number Diff line change
@@ -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. |
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down