Skip to content

Commit 9b651ea

Browse files
committed
C++: Fix mapping of sources from Expr to Node
The code contained the remains of how `isUserInput` in `Security.qll` used to be ported to IR. It's wrong to use that port since many queries call `userInput` directly to get the "cause" string.
1 parent d7e8ea7 commit 9b651ea

File tree

1 file changed

+13
-28
lines changed

1 file changed

+13
-28
lines changed

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

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,31 +21,19 @@ private predicate predictableInstruction(Instruction instr) {
2121
predictableInstruction(instr.(UnaryInstruction).getUnary())
2222
}
2323

24+
private DataFlow::Node getNodeForSource(Expr source) {
25+
isUserInput(source, _) and
26+
(
27+
result = DataFlow::exprNode(source)
28+
or
29+
result = DataFlow::definitionByReferenceNode(source)
30+
)
31+
}
32+
2433
private class DefaultTaintTrackingCfg extends DataFlow::Configuration {
2534
DefaultTaintTrackingCfg() { this = "DefaultTaintTrackingCfg" }
2635

27-
override predicate isSource(DataFlow::Node source) {
28-
exists(CallInstruction ci, WriteSideEffectInstruction wsei |
29-
userInputArgument(ci.getConvertedResultExpression(), wsei.getIndex()) and
30-
source.asInstruction() = wsei and
31-
wsei.getPrimaryInstruction() = ci
32-
)
33-
or
34-
userInputReturned(source.asExpr())
35-
or
36-
isUserInput(source.asExpr(), _)
37-
or
38-
source.asExpr() instanceof EnvironmentRead
39-
or
40-
source
41-
.asInstruction()
42-
.(LoadInstruction)
43-
.getSourceAddress()
44-
.(VariableAddressInstruction)
45-
.getASTVariable()
46-
.hasName("argv") and
47-
source.asInstruction().getEnclosingFunction().hasGlobalName("main")
48-
}
36+
override predicate isSource(DataFlow::Node source) { source = getNodeForSource(_) }
4937

5038
override predicate isSink(DataFlow::Node sink) { any() }
5139

@@ -59,7 +47,7 @@ private class DefaultTaintTrackingCfg extends DataFlow::Configuration {
5947
private class ToGlobalVarTaintTrackingCfg extends DataFlow::Configuration {
6048
ToGlobalVarTaintTrackingCfg() { this = "GlobalVarTaintTrackingCfg" }
6149

62-
override predicate isSource(DataFlow::Node source) { isUserInput(source.asExpr(), _) }
50+
override predicate isSource(DataFlow::Node source) { source = getNodeForSource(_) }
6351

6452
override predicate isSink(DataFlow::Node sink) {
6553
exists(GlobalOrNamespaceVariable gv | writesVariable(sink.asInstruction(), gv))
@@ -283,10 +271,7 @@ private Element adjustedSink(DataFlow::Node sink) {
283271

284272
predicate tainted(Expr source, Element tainted) {
285273
exists(DefaultTaintTrackingCfg cfg, DataFlow::Node sink |
286-
cfg.hasFlow(DataFlow::exprNode(source), sink)
287-
or
288-
cfg.hasFlow(DataFlow::definitionByReferenceNode(source), sink)
289-
|
274+
cfg.hasFlow(getNodeForSource(source), sink) and
290275
tainted = adjustedSink(sink)
291276
)
292277
}
@@ -299,7 +284,7 @@ predicate taintedIncludingGlobalVars(Expr source, Element tainted, string global
299284
ToGlobalVarTaintTrackingCfg toCfg, FromGlobalVarTaintTrackingCfg fromCfg, DataFlow::Node store,
300285
GlobalOrNamespaceVariable global, DataFlow::Node load, DataFlow::Node sink
301286
|
302-
toCfg.hasFlow(DataFlow::exprNode(source), store) and
287+
toCfg.hasFlow(getNodeForSource(source), store) and
303288
store
304289
.asInstruction()
305290
.(StoreInstruction)

0 commit comments

Comments
 (0)