Skip to content

Commit 2b10cd6

Browse files
author
Robert Marsh
authored
Merge pull request #2737 from jbj/DefaultTaintTracking-indirect-parameters
C++: Interprocedural indirections in DefaultTaintTracking.qll
2 parents 3a7845e + e2da98a commit 2b10cd6

File tree

3 files changed

+34
-19
lines changed

3 files changed

+34
-19
lines changed

cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,22 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
151151
// from `a`.
152152
i2.(PointerAddInstruction).getLeft() = i1
153153
or
154+
// Until we have from through indirections across calls, we'll take flow out
155+
// of the parameter and into its indirection.
156+
exists(IRFunction f, Parameter parameter |
157+
i1 = getInitializeParameter(f, parameter) and
158+
i2 = getInitializeIndirection(f, parameter)
159+
)
160+
or
161+
// Until we have flow through indirections across calls, we'll take flow out
162+
// of the indirection and into the argument.
163+
// When we get proper flow through indirections across calls, this code can be
164+
// moved to `adjusedSink` or possibly into the `DataFlow::ExprNode` class.
165+
exists(ReadSideEffectInstruction read |
166+
read.getAnOperand().(SideEffectOperand).getAnyDef() = i1 and
167+
read.getArgumentDef() = i2
168+
)
169+
or
154170
// Flow from argument to return value
155171
i2 =
156172
any(CallInstruction call |
@@ -176,6 +192,18 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
176192
)
177193
}
178194

195+
pragma[noinline]
196+
private InitializeIndirectionInstruction getInitializeIndirection(IRFunction f, Parameter p) {
197+
result.getParameter() = p and
198+
result.getEnclosingIRFunction() = f
199+
}
200+
201+
pragma[noinline]
202+
private InitializeParameterInstruction getInitializeParameter(IRFunction f, Parameter p) {
203+
result.getParameter() = p and
204+
result.getEnclosingIRFunction() = f
205+
}
206+
179207
/**
180208
* Get an instruction that goes into argument `argumentIndex` of `call`. This
181209
* can be either directly or through one pointer indirection.
@@ -273,23 +301,6 @@ private Element adjustedSink(DataFlow::Node sink) {
273301
// For compatibility, send flow into a `NotExpr` even if it's part of a
274302
// short-circuiting condition and thus might get skipped.
275303
result.(NotExpr).getOperand() = sink.asExpr()
276-
or
277-
// For compatibility, send flow from argument read side effects to their
278-
// corresponding argument expression
279-
exists(IndirectReadSideEffectInstruction read |
280-
read.getAnOperand().(SideEffectOperand).getAnyDef() = sink.asInstruction() and
281-
read.getArgumentDef().getUnconvertedResultExpression() = result
282-
)
283-
or
284-
exists(BufferReadSideEffectInstruction read |
285-
read.getAnOperand().(SideEffectOperand).getAnyDef() = sink.asInstruction() and
286-
read.getArgumentDef().getUnconvertedResultExpression() = result
287-
)
288-
or
289-
exists(SizedBufferReadSideEffectInstruction read |
290-
read.getAnOperand().(SideEffectOperand).getAnyDef() = sink.asInstruction() and
291-
read.getArgumentDef().getUnconvertedResultExpression() = result
292-
)
293304
}
294305

295306
predicate tainted(Expr source, Element tainted) {

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,18 @@
2121
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:8:22:33 | (const char *)... |
2222
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:20:22:25 | call to getenv |
2323
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:20:22:32 | (const char *)... |
24+
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | (const char *)... |
25+
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | array to pointer conversion |
2426
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | buf |
2527
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
28+
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:31:40:31:53 | dotted_address |
2629
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:32:11:32:26 | p#0 |
2730
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:11:38:21 | env_pointer |
2831
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:25:38:30 | call to getenv |
2932
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:25:38:37 | (void *)... |
3033
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:22:39:22 | a |
3134
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:26:39:34 | call to inet_addr |
35+
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:36:39:61 | (const char *)... |
3236
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:50:39:61 | & ... |
3337
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:40:10:40:10 | a |
3438
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:3:21:3:22 | s1 | AST only |
66
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:21:8:21:10 | buf | AST only |
77
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:15:22:17 | buf | AST only |
8-
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:31:40:31:53 | dotted_address | AST only |
9-
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:36:39:61 | (const char *)... | AST only |
8+
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | (const char *)... | IR only |
9+
| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | array to pointer conversion | IR only |
1010
| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:51:39:61 | env_pointer | AST only |
1111
| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p | IR only |
1212
| test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:11:104:20 | (...) | IR only |

0 commit comments

Comments
 (0)