Repository navigation
C++: wire up models library to DefaultTaintTracking #2657
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,8 @@ private import semmle.code.cpp.ir.dataflow.DataFlow | |
| private import semmle.code.cpp.ir.dataflow.DataFlow2 | ||
| private import semmle.code.cpp.ir.IR | ||
| private import semmle.code.cpp.ir.dataflow.internal.DataFlowDispatch as Dispatch | ||
| private import semmle.code.cpp.models.interfaces.Taint | ||
| private import semmle.code.cpp.models.interfaces.DataFlow | ||
|
|
||
| /** | ||
| * A predictable instruction is one where an external user can predict | ||
|
|
@@ -159,18 +161,64 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) { | |
| // This is part of the translation of `a[i]`, where we want taint to flow | ||
| // from `a`. | ||
| i2.(PointerAddInstruction).getLeft() = i1 | ||
| or | ||
| // Flow from argument to return value | ||
| i2 = any(CallInstruction call | | ||
| exists(int indexIn | | ||
| modelTaintToReturnValue(call.getStaticCallTarget(), indexIn) and | ||
| i1 = call.getPositionalArgument(indexIn) | ||
|
Contributor
There was a problem hiding this comment. Does this also need to account for indirect parameters via
Contributor
Author
There was a problem hiding this comment. Yes it should. I've added three commits to test and implement that functionality. The test I came up with is very contrived since there's almost no flow to indirections without #2667. Hopefully this will be better soon. |
||
| ) | ||
| ) | ||
| or | ||
| // Flow from input argument to output argument | ||
| // TODO: This won't work in practice as long as all aliased memory is tracked | ||
| // together in a single virtual variable. | ||
| // TODO: Will this work on the test for `TaintedPath.ql`, where the output arg | ||
| // is a pointer addition expression? | ||
| i2 = any(WriteSideEffectInstruction outNode | | ||
| exists(CallInstruction call, int indexIn, int indexOut | | ||
| modelTaintToParameter(call.getStaticCallTarget(), indexIn, indexOut) and | ||
| i1 = call.getPositionalArgument(indexIn) and | ||
|
jbj marked this conversation as resolved.
Resolved
|
||
| outNode.getIndex() = indexOut and | ||
| outNode.getPrimaryInstruction() = call | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
| private predicate modelTaintToParameter(Function f, int parameterIn, int parameterOut) { | ||
| exists( FunctionInput modelIn, FunctionOutput modelOut | | ||
| f.(TaintFunction).hasTaintFlow(modelIn, modelOut) and | ||
| (modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and | ||
| modelOut.isParameterDeref(parameterOut) | ||
| ) | ||
| } | ||
|
|
||
| private predicate modelTaintToReturnValue(Function f, int parameterIn) { | ||
| // Taint flow from parameter to return value | ||
| exists(FunctionInput modelIn, FunctionOutput modelOut | | ||
| f.(TaintFunction).hasTaintFlow(modelIn, modelOut) and | ||
| (modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and | ||
| (modelOut.isReturnValue() or modelOut.isReturnValueDeref()) | ||
| ) | ||
| or | ||
| // Data flow (not taint flow) to where the return value points. For the time | ||
| // being we will conflate pointers and objects in taint tracking. | ||
| exists(FunctionInput modelIn, FunctionOutput modelOut | | ||
| f.(DataFlowFunction).hasDataFlow(modelIn, modelOut) and | ||
| (modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and | ||
| modelOut.isReturnValueDeref() | ||
| ) | ||
| or | ||
| // Taint flow from one argument to another and data flow from an argument to a | ||
| // return value. This happens in functions like `strcat` and `memcpy`. We | ||
| // could model this flow in two separate steps, but that would add reverse | ||
| // flow from the write side-effect to the call instruction, which may not be | ||
| // desirable. | ||
| exists(int parameterMid, InParameter modelMid, OutReturnValue returnOut | | ||
| modelTaintToParameter(f, parameterIn, parameterMid) and | ||
| modelMid.isParameter(parameterMid) and | ||
| f.(DataFlowFunction).hasDataFlow(modelMid, returnOut) | ||
| ) | ||
| } | ||
|
|
||
| private Element adjustedSink(DataFlow::Node sink) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| int atoi(const char *nptr); | ||
| char *getenv(const char *name); | ||
| char *strcat(char * s1, const char * s2); | ||
|
|
||
| char *strdup(const char *); | ||
| char *_strdup(const char *); | ||
| char *unmodeled_function(const char *); | ||
|
|
||
| void sink(const char *); | ||
| void sink(int); | ||
|
|
||
| int main(int argc, char *argv[]) { | ||
| int taintedInt = atoi(getenv("VAR")); | ||
| taintedInt++; // BUG: `taintedInt` isn't marked as tainted. Only `++` is. | ||
|
|
||
| sink(_strdup(getenv("VAR"))); | ||
| sink(strdup(getenv("VAR"))); | ||
| sink(unmodeled_function(getenv("VAR"))); | ||
|
|
||
| char untainted_buf[100] = ""; | ||
| char buf[100] = "VAR = "; | ||
| sink(strcat(buf, getenv("VAR"))); | ||
|
|
||
| sink(buf); // BUG: no taint | ||
| sink(untainted_buf); // the two buffers would be conflated if we added flow through partial chi inputs | ||
|
|
||
| return 0; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| | defaulttainttracking.cpp:13:25:13:30 | call to getenv | defaulttainttracking.cpp:1:22:1:25 | nptr | | ||
| | defaulttainttracking.cpp:13:25:13:30 | call to getenv | defaulttainttracking.cpp:13:7:13:16 | taintedInt | | ||
| | defaulttainttracking.cpp:13:25:13:30 | call to getenv | defaulttainttracking.cpp:13:20:13:23 | call to atoi | | ||
| | defaulttainttracking.cpp:13:25:13:30 | call to getenv | defaulttainttracking.cpp:13:25:13:30 | call to getenv | | ||
| | defaulttainttracking.cpp:13:25:13:30 | call to getenv | defaulttainttracking.cpp:13:25:13:37 | (const char *)... | | ||
| | defaulttainttracking.cpp:13:25:13:30 | call to getenv | defaulttainttracking.cpp:14:3:14:14 | ... ++ | | ||
| | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:6:15:6:24 | p#0 | | ||
| | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | | ||
| | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:8:16:14 | call to _strdup | | ||
| | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:8:16:29 | (const char *)... | | ||
| | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:16:16:21 | call to getenv | | ||
| | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:16:16:28 | (const char *)... | | ||
| | defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:5:14:5:23 | p#0 | | ||
| | defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | | ||
| | defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:8:17:13 | call to strdup | | ||
| | defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:8:17:28 | (const char *)... | | ||
| | defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:15:17:20 | call to getenv | | ||
| | defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:15:17:27 | (const char *)... | | ||
| | defaulttainttracking.cpp:18:27:18:32 | call to getenv | defaulttainttracking.cpp:7:26:7:35 | p#0 | | ||
| | defaulttainttracking.cpp:18:27:18:32 | call to getenv | defaulttainttracking.cpp:18:27:18:32 | call to getenv | | ||
| | defaulttainttracking.cpp:18:27:18:32 | call to getenv | defaulttainttracking.cpp:18:27:18:39 | (const char *)... | | ||
| | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:3:38:3:39 | s2 | | ||
| | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | | ||
| | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:8:22:13 | call to strcat | | ||
| | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:8:22:33 | (const char *)... | | ||
| | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:20:22:25 | call to getenv | | ||
| | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:20:22:32 | (const char *)... | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import semmle.code.cpp.ir.dataflow.DefaultTaintTracking | ||
|
|
||
| from Expr source, Element tainted | ||
| where tainted(source, tainted) | ||
| select source, tainted |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to remove this TODO comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because I don't think it belongs here. It probably belongs in the IR data-flow library and the IR itself, which means it should be in a Jira ticket -- I've opened CPP-490.