Skip to content

Commit bd0a196

Browse files
hvitvedhvitved
committed
Java: Update data-flow caching
1 parent befc80b commit bd0a196

File tree

4 files changed

+105
-111
lines changed

4 files changed

+105
-111
lines changed

‎java/ql/src/semmle/code/java/dataflow/internal/DataFlowNodes.qll

Lines changed: 44 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -4,70 +4,47 @@ private import semmle.code.java.dataflow.FlowSummary
44
private import semmle.code.java.dataflow.TypeFlow
55
private import DataFlowPrivate
66
private import FlowSummaryImpl as FlowSummaryImpl
7+
private import DataFlowImplCommon as DataFlowImplCommon
78

89
cached
9-
private module Cached {
10-
cached
11-
newtype TNode =
12-
TExprNode(Expr e) {
13-
not e.getType() instanceof VoidType and
14-
not e.getParent*() instanceof Annotation
15-
} or
16-
TExplicitParameterNode(Parameter p) {
17-
exists(p.getCallable().getBody()) or p.getCallable() instanceof SummarizedCallable
18-
} or
19-
TImplicitVarargsArray(Call c) {
20-
c.getCallee().isVarargs() and
21-
not exists(Argument arg | arg.getCall() = c and arg.isExplicitVarargsArray())
22-
} or
23-
TInstanceParameterNode(Callable c) {
24-
(exists(c.getBody()) or c instanceof SummarizedCallable) and
25-
not c.isStatic()
26-
} or
27-
TImplicitInstanceAccess(InstanceAccessExt ia) { not ia.isExplicit(_) } or
28-
TMallocNode(ClassInstanceExpr cie) or
29-
TExplicitExprPostUpdate(Expr e) {
30-
explicitInstanceArgument(_, e)
31-
or
32-
e instanceof Argument and not e.getType() instanceof ImmutableType
33-
or
34-
exists(FieldAccess fa | fa.getField() instanceof InstanceField and e = fa.getQualifier())
35-
or
36-
exists(ArrayAccess aa | e = aa.getArray())
37-
} or
38-
TImplicitExprPostUpdate(InstanceAccessExt ia) {
39-
implicitInstanceArgument(_, ia)
40-
or
41-
exists(FieldAccess fa |
42-
fa.getField() instanceof InstanceField and ia.isImplicitFieldQualifier(fa)
43-
)
44-
} or
45-
TSummaryInternalNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) {
46-
FlowSummaryImpl::Private::summaryNodeRange(c, state)
47-
}
48-
49-
cached
50-
predicate summaryOutNodeCached(DataFlowCall c, Node out) {
51-
FlowSummaryImpl::Private::summaryOutNode(c, out, _)
52-
}
53-
54-
cached
55-
predicate summaryArgumentNodeCached(DataFlowCall c, Node arg, int i) {
56-
FlowSummaryImpl::Private::summaryArgumentNode(c, arg, i)
57-
}
58-
59-
cached
60-
predicate summaryPostUpdateNodeCached(Node post, ParameterNode pre) {
61-
FlowSummaryImpl::Private::summaryPostUpdateNode(post, pre)
62-
}
63-
64-
cached
65-
predicate summaryReturnNodeCached(Node ret) {
66-
FlowSummaryImpl::Private::summaryReturnNode(ret, _)
10+
newtype TNode =
11+
TExprNode(Expr e) {
12+
DataFlowImplCommon::forceCachingInSameStage() and
13+
not e.getType() instanceof VoidType and
14+
not e.getParent*() instanceof Annotation
15+
} or
16+
TExplicitParameterNode(Parameter p) {
17+
exists(p.getCallable().getBody()) or p.getCallable() instanceof SummarizedCallable
18+
} or
19+
TImplicitVarargsArray(Call c) {
20+
c.getCallee().isVarargs() and
21+
not exists(Argument arg | arg.getCall() = c and arg.isExplicitVarargsArray())
22+
} or
23+
TInstanceParameterNode(Callable c) {
24+
(exists(c.getBody()) or c instanceof SummarizedCallable) and
25+
not c.isStatic()
26+
} or
27+
TImplicitInstanceAccess(InstanceAccessExt ia) { not ia.isExplicit(_) } or
28+
TMallocNode(ClassInstanceExpr cie) or
29+
TExplicitExprPostUpdate(Expr e) {
30+
explicitInstanceArgument(_, e)
31+
or
32+
e instanceof Argument and not e.getType() instanceof ImmutableType
33+
or
34+
exists(FieldAccess fa | fa.getField() instanceof InstanceField and e = fa.getQualifier())
35+
or
36+
exists(ArrayAccess aa | e = aa.getArray())
37+
} or
38+
TImplicitExprPostUpdate(InstanceAccessExt ia) {
39+
implicitInstanceArgument(_, ia)
40+
or
41+
exists(FieldAccess fa |
42+
fa.getField() instanceof InstanceField and ia.isImplicitFieldQualifier(fa)
43+
)
44+
} or
45+
TSummaryInternalNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) {
46+
FlowSummaryImpl::Private::summaryNodeRange(c, state)
6747
}
68-
}
69-
70-
private import Cached
7148

7249
private predicate explicitInstanceArgument(Call call, Expr instarg) {
7350
call instanceof MethodAccess and
@@ -404,13 +381,15 @@ module Private {
404381
override string toString() { result = "[summary] " + state + " in " + c }
405382

406383
/** Holds if this summary node is the `i`th argument of `call`. */
407-
predicate isArgumentOf(DataFlowCall call, int i) { summaryArgumentNodeCached(call, this, i) }
384+
predicate isArgumentOf(DataFlowCall call, int i) {
385+
FlowSummaryImpl::Private::summaryArgumentNode(call, this, i)
386+
}
408387

409388
/** Holds if this summary node is a return node. */
410-
predicate isReturn() { summaryReturnNodeCached(this) }
389+
predicate isReturn() { FlowSummaryImpl::Private::summaryReturnNode(this, _) }
411390

412391
/** Holds if this summary node is an out node for `call`. */
413-
predicate isOut(DataFlowCall call) { summaryOutNodeCached(call, this) }
392+
predicate isOut(DataFlowCall call) { FlowSummaryImpl::Private::summaryOutNode(call, this, _) }
414393
}
415394

416395
SummaryNode getSummaryNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) {
@@ -439,7 +418,7 @@ private class MallocNode extends Node, TMallocNode {
439418
private class SummaryPostUpdateNode extends SummaryNode, PostUpdateNode {
440419
private Node pre;
441420

442-
SummaryPostUpdateNode() { summaryPostUpdateNodeCached(this, pre) }
421+
SummaryPostUpdateNode() { FlowSummaryImpl::Private::summaryPostUpdateNode(this, pre) }
443422

444423
override Node getPreUpdateNode() { result = pre }
445424
}

‎java/ql/src/semmle/code/java/dataflow/internal/DataFlowPrivate.qll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,6 @@ private class ConstantBooleanArgumentNode extends ArgumentNode, ExprNode {
284284
/**
285285
* Holds if the node `n` is unreachable when the call context is `call`.
286286
*/
287-
cached
288287
predicate isUnreachableInCall(Node n, DataFlowCall call) {
289288
exists(
290289
ExplicitParameterNode paramNode, ConstantBooleanArgumentNode arg, SsaImplicitInit param,

‎java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ private import semmle.code.java.dataflow.FlowSteps
1111
private import semmle.code.java.dataflow.FlowSummary
1212
import semmle.code.java.dataflow.InstanceAccess
1313
private import FlowSummaryImpl as FlowSummaryImpl
14+
private import TaintTrackingUtil as TaintTrackingUtil
1415
import DataFlowNodes::Public
1516

1617
/** Holds if `n` is an access to an unqualified `this` at `cfgnode`. */
@@ -112,6 +113,7 @@ predicate localFlowStep(Node node1, Node node2) {
112113
*/
113114
cached
114115
predicate simpleLocalFlowStep(Node node1, Node node2) {
116+
TaintTrackingUtil::forceCachingInSameStage() and
115117
// Variable flow steps through adjacent def-use and use-use pairs.
116118
exists(SsaExplicitUpdate upd |
117119
upd.getDefiningExpr().(VariableAssign).getSource() = node1.asExpr() or

‎java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

Lines changed: 59 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -29,56 +29,70 @@ predicate localExprTaint(Expr src, Expr sink) {
2929
localTaint(DataFlow::exprNode(src), DataFlow::exprNode(sink))
3030
}
3131

32-
/**
33-
* Holds if taint can flow in one local step from `src` to `sink`.
34-
*/
35-
predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) {
36-
DataFlow::localFlowStep(src, sink) or
37-
localAdditionalTaintStep(src, sink) or
38-
// Simple flow through library code is included in the exposed local
39-
// step relation, even though flow is technically inter-procedural
40-
FlowSummaryImpl::Private::Steps::summaryThroughStep(src, sink, false)
41-
}
32+
cached
33+
private module Cached {
34+
private import DataFlowImplCommon as DataFlowImplCommon
4235

43-
/**
44-
* Holds if taint can flow in one local step from `src` to `sink` excluding
45-
* local data flow steps. That is, `src` and `sink` are likely to represent
46-
* different objects.
47-
*/
48-
predicate localAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
49-
localAdditionalTaintExprStep(src.asExpr(), sink.asExpr())
50-
or
51-
localAdditionalTaintUpdateStep(src.asExpr(),
52-
sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr())
53-
or
54-
exists(Argument arg |
55-
src.asExpr() = arg and
56-
arg.isVararg() and
57-
sink.(DataFlow::ImplicitVarargsArray).getCall() = arg.getCall()
58-
)
59-
or
60-
FlowSummaryImpl::Private::Steps::summaryLocalStep(src, sink, false)
61-
}
36+
cached
37+
predicate forceCachingInSameStage() { DataFlowImplCommon::forceCachingInSameStage() }
6238

63-
/**
64-
* Holds if the additional step from `src` to `sink` should be included in all
65-
* global taint flow configurations.
66-
*/
67-
predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
68-
localAdditionalTaintStep(src, sink) or
69-
any(AdditionalTaintStep a).step(src, sink)
70-
}
39+
/**
40+
* Holds if taint can flow in one local step from `src` to `sink`.
41+
*/
42+
cached
43+
predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) {
44+
DataFlow::localFlowStep(src, sink) or
45+
localAdditionalTaintStep(src, sink) or
46+
// Simple flow through library code is included in the exposed local
47+
// step relation, even though flow is technically inter-procedural
48+
FlowSummaryImpl::Private::Steps::summaryThroughStep(src, sink, false)
49+
}
7150

72-
/**
73-
* Holds if `node` should be a sanitizer in all global taint flow configurations
74-
* but not in local taint.
75-
*/
76-
predicate defaultTaintSanitizer(DataFlow::Node node) {
77-
// Ignore paths through test code.
78-
node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass or
79-
node.asExpr() instanceof ValidatedVariableAccess
51+
/**
52+
* Holds if taint can flow in one local step from `src` to `sink` excluding
53+
* local data flow steps. That is, `src` and `sink` are likely to represent
54+
* different objects.
55+
*/
56+
cached
57+
predicate localAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
58+
localAdditionalTaintExprStep(src.asExpr(), sink.asExpr())
59+
or
60+
localAdditionalTaintUpdateStep(src.asExpr(),
61+
sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr())
62+
or
63+
exists(Argument arg |
64+
src.asExpr() = arg and
65+
arg.isVararg() and
66+
sink.(DataFlow::ImplicitVarargsArray).getCall() = arg.getCall()
67+
)
68+
or
69+
FlowSummaryImpl::Private::Steps::summaryLocalStep(src, sink, false)
70+
}
71+
72+
/**
73+
* Holds if the additional step from `src` to `sink` should be included in all
74+
* global taint flow configurations.
75+
*/
76+
cached
77+
predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
78+
localAdditionalTaintStep(src, sink) or
79+
any(AdditionalTaintStep a).step(src, sink)
80+
}
81+
82+
/**
83+
* Holds if `node` should be a sanitizer in all global taint flow configurations
84+
* but not in local taint.
85+
*/
86+
cached
87+
predicate defaultTaintSanitizer(DataFlow::Node node) {
88+
// Ignore paths through test code.
89+
node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass or
90+
node.asExpr() instanceof ValidatedVariableAccess
91+
}
8092
}
8193

94+
import Cached
95+
8296
/**
8397
* Holds if taint can flow in one local step from `src` to `sink` excluding
8498
* local data flow steps. That is, `src` and `sink` are likely to represent

0 commit comments

Comments
 (0)