Skip to content

Commit 253f932

Browse files
hvitvedhvitved
committed
Python: Use data flow consistency checks from shared pack
1 parent 9af706c commit 253f932

File tree

3 files changed

+35
-325
lines changed

3 files changed

+35
-325
lines changed

‎config/identical-files.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,6 @@
5555
"ruby/ql/lib/codeql/ruby/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
5656
"swift/ql/lib/codeql/swift/dataflow/internal/tainttracking1/TaintTrackingImpl.qll"
5757
],
58-
"DataFlow Java/C++/C#/Python/Ruby/Swift Consistency checks": [
59-
"python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll"
60-
],
6158
"DataFlow Java/C#/Go/Ruby/Python/Swift Flow Summaries": [
6259
"java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll",
6360
"csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll",

‎python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll

Lines changed: 33 additions & 280 deletions
Original file line numberDiff line numberDiff line change
@@ -3,297 +3,50 @@
33
* data-flow classes and predicates.
44
*/
55

6-
private import DataFlowImplSpecific::Private
7-
private import DataFlowImplSpecific::Public
8-
private import tainttracking1.TaintTrackingParameter::Private
9-
private import tainttracking1.TaintTrackingParameter::Public
6+
private import python
7+
private import DataFlowImplSpecific
8+
private import TaintTrackingImplSpecific
9+
private import codeql.dataflow.internal.DataFlowImplConsistency
1010

11-
module Consistency {
12-
private newtype TConsistencyConfiguration = MkConsistencyConfiguration()
11+
private module Input implements InputSig<PythonDataFlow> {
12+
private import Private
13+
private import Public
1314

14-
/** A class for configuring the consistency queries. */
15-
class ConsistencyConfiguration extends TConsistencyConfiguration {
16-
string toString() { none() }
17-
18-
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
19-
predicate uniqueEnclosingCallableExclude(Node n) { none() }
20-
21-
/** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */
22-
predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() }
23-
24-
/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
25-
predicate uniqueNodeLocationExclude(Node n) { none() }
26-
27-
/** Holds if `n` should be excluded from the consistency test `missingLocation`. */
28-
predicate missingLocationExclude(Node n) { none() }
29-
30-
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
31-
predicate postWithInFlowExclude(Node n) { none() }
32-
33-
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
34-
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
35-
36-
/** Holds if `n` should be excluded from the consistency test `reverseRead`. */
37-
predicate reverseReadExclude(Node n) { none() }
38-
39-
/** Holds if `n` should be excluded from the consistency test `postHasUniquePre`. */
40-
predicate postHasUniquePreExclude(PostUpdateNode n) { none() }
41-
42-
/** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */
43-
predicate uniquePostUpdateExclude(Node n) { none() }
44-
45-
/** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */
46-
predicate viableImplInCallContextTooLargeExclude(
47-
DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
48-
) {
49-
none()
50-
}
51-
52-
/** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodeAtPosition`. */
53-
predicate uniqueParameterNodeAtPositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) {
54-
none()
55-
}
56-
57-
/** Holds if `(c, pos, p)` should be excluded from the consistency test `uniqueParameterNodePosition`. */
58-
predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) {
59-
none()
60-
}
61-
62-
/** Holds if `n` should be excluded from the consistency test `identityLocalStep`. */
63-
predicate identityLocalStepExclude(Node n) { none() }
64-
}
65-
66-
private class RelevantNode extends Node {
67-
RelevantNode() {
68-
this instanceof ArgumentNode or
69-
this instanceof ParameterNode or
70-
this instanceof ReturnNode or
71-
this = getAnOutNode(_, _) or
72-
simpleLocalFlowStep(this, _) or
73-
simpleLocalFlowStep(_, this) or
74-
jumpStep(this, _) or
75-
jumpStep(_, this) or
76-
storeStep(this, _, _) or
77-
storeStep(_, _, this) or
78-
readStep(this, _, _) or
79-
readStep(_, _, this) or
80-
defaultAdditionalTaintStep(this, _) or
81-
defaultAdditionalTaintStep(_, this)
82-
}
83-
}
84-
85-
query predicate uniqueEnclosingCallable(Node n, string msg) {
86-
exists(int c |
87-
n instanceof RelevantNode and
88-
c = count(nodeGetEnclosingCallable(n)) and
89-
c != 1 and
90-
not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and
91-
msg = "Node should have one enclosing callable but has " + c + "."
92-
)
93-
}
94-
95-
query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) {
96-
exists(int c |
97-
c = count(call.getEnclosingCallable()) and
98-
c != 1 and
99-
not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and
100-
msg = "Call should have one enclosing callable but has " + c + "."
101-
)
102-
}
103-
104-
query predicate uniqueType(Node n, string msg) {
105-
exists(int c |
106-
n instanceof RelevantNode and
107-
c = count(getNodeType(n)) and
108-
c != 1 and
109-
msg = "Node should have one type but has " + c + "."
110-
)
111-
}
112-
113-
query predicate uniqueNodeLocation(Node n, string msg) {
114-
exists(int c |
115-
c =
116-
count(string filepath, int startline, int startcolumn, int endline, int endcolumn |
117-
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
118-
) and
119-
c != 1 and
120-
not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and
121-
msg = "Node should have one location but has " + c + "."
122-
)
123-
}
124-
125-
query predicate missingLocation(string msg) {
126-
exists(int c |
127-
c =
128-
strictcount(Node n |
129-
not n.hasLocationInfo(_, _, _, _, _) and
130-
not any(ConsistencyConfiguration conf).missingLocationExclude(n)
131-
) and
132-
msg = "Nodes without location: " + c
133-
)
134-
}
135-
136-
query predicate uniqueNodeToString(Node n, string msg) {
137-
exists(int c |
138-
c = count(n.toString()) and
139-
c != 1 and
140-
msg = "Node should have one toString but has " + c + "."
141-
)
142-
}
143-
144-
query predicate missingToString(string msg) {
145-
exists(int c |
146-
c = strictcount(Node n | not exists(n.toString())) and
147-
msg = "Nodes without toString: " + c
148-
)
149-
}
150-
151-
query predicate parameterCallable(ParameterNode p, string msg) {
152-
exists(DataFlowCallable c | isParameterNode(p, c, _) and c != nodeGetEnclosingCallable(p)) and
153-
msg = "Callable mismatch for parameter."
154-
}
155-
156-
query predicate localFlowIsLocal(Node n1, Node n2, string msg) {
157-
simpleLocalFlowStep(n1, n2) and
158-
nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
159-
msg = "Local flow step does not preserve enclosing callable."
160-
}
161-
162-
query predicate readStepIsLocal(Node n1, Node n2, string msg) {
163-
readStep(n1, _, n2) and
164-
nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
165-
msg = "Read step does not preserve enclosing callable."
166-
}
167-
168-
query predicate storeStepIsLocal(Node n1, Node n2, string msg) {
169-
storeStep(n1, _, n2) and
170-
nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
171-
msg = "Store step does not preserve enclosing callable."
172-
}
173-
174-
private DataFlowType typeRepr() { result = getNodeType(_) }
175-
176-
query predicate compatibleTypesReflexive(DataFlowType t, string msg) {
177-
t = typeRepr() and
178-
not compatibleTypes(t, t) and
179-
msg = "Type compatibility predicate is not reflexive."
180-
}
181-
182-
query predicate unreachableNodeCCtx(Node n, DataFlowCall call, string msg) {
183-
isUnreachableInCall(n, call) and
184-
exists(DataFlowCallable c |
185-
c = nodeGetEnclosingCallable(n) and
186-
not viableCallable(call) = c
187-
) and
188-
msg = "Call context for isUnreachableInCall is inconsistent with call graph."
189-
}
190-
191-
query predicate localCallNodes(DataFlowCall call, Node n, string msg) {
192-
(
193-
n = getAnOutNode(call, _) and
194-
msg = "OutNode and call does not share enclosing callable."
195-
or
196-
n.(ArgumentNode).argumentOf(call, _) and
197-
msg = "ArgumentNode and call does not share enclosing callable."
198-
) and
199-
nodeGetEnclosingCallable(n) != call.getEnclosingCallable()
200-
}
201-
202-
// This predicate helps the compiler forget that in some languages
203-
// it is impossible for a result of `getPreUpdateNode` to be an
204-
// instance of `PostUpdateNode`.
205-
private Node getPre(PostUpdateNode n) {
206-
result = n.getPreUpdateNode()
15+
predicate argHasPostUpdateExclude(ArgumentNode n) {
16+
exists(ArgumentPosition apos | n.argumentOf(_, apos) and apos.isStarArgs(_))
20717
or
208-
none()
18+
exists(ArgumentPosition apos | n.argumentOf(_, apos) and apos.isDictSplat())
20919
}
21020

211-
query predicate postIsNotPre(PostUpdateNode n, string msg) {
212-
getPre(n) = n and
213-
msg = "PostUpdateNode should not equal its pre-update node."
21+
predicate reverseReadExclude(Node n) {
22+
// since `self`/`cls` parameters can be marked as implicit argument to `super()`,
23+
// they will have PostUpdateNodes. We have a read-step from the synthetic `**kwargs`
24+
// parameter, but dataflow-consistency queries should _not_ complain about there not
25+
// being a post-update node for the synthetic `**kwargs` parameter.
26+
n instanceof SynthDictSplatParameterNode
21427
}
21528

216-
query predicate postHasUniquePre(PostUpdateNode n, string msg) {
217-
not any(ConsistencyConfiguration conf).postHasUniquePreExclude(n) and
218-
exists(int c |
219-
c = count(n.getPreUpdateNode()) and
220-
c != 1 and
221-
msg = "PostUpdateNode should have one pre-update node but has " + c + "."
29+
predicate uniqueParameterNodePositionExclude(DataFlowCallable c, ParameterPosition pos, Node p) {
30+
// For normal parameters that can both be passed as positional arguments or keyword
31+
// arguments, we currently have parameter positions for both cases..
32+
//
33+
// TODO: Figure out how bad breaking this consistency check is
34+
exists(Function func, Parameter param |
35+
c.getScope() = func and
36+
p = parameterNode(param) and
37+
c.getParameter(pos) = p and
38+
param = func.getArg(_) and
39+
param = func.getArgByName(_)
22240
)
22341
}
22442

225-
query predicate uniquePostUpdate(Node n, string msg) {
226-
not any(ConsistencyConfiguration conf).uniquePostUpdateExclude(n) and
227-
1 < strictcount(PostUpdateNode post | post.getPreUpdateNode() = n) and
228-
msg = "Node has multiple PostUpdateNodes."
229-
}
230-
231-
query predicate postIsInSameCallable(PostUpdateNode n, string msg) {
232-
nodeGetEnclosingCallable(n) != nodeGetEnclosingCallable(n.getPreUpdateNode()) and
233-
msg = "PostUpdateNode does not share callable with its pre-update node."
234-
}
235-
236-
private predicate hasPost(Node n) { exists(PostUpdateNode post | post.getPreUpdateNode() = n) }
237-
238-
query predicate reverseRead(Node n, string msg) {
239-
exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and
240-
not any(ConsistencyConfiguration conf).reverseReadExclude(n) and
241-
msg = "Origin of readStep is missing a PostUpdateNode."
242-
}
243-
244-
query predicate argHasPostUpdate(ArgumentNode n, string msg) {
245-
not hasPost(n) and
246-
not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and
247-
msg = "ArgumentNode is missing PostUpdateNode."
248-
}
249-
250-
// This predicate helps the compiler forget that in some languages
251-
// it is impossible for a `PostUpdateNode` to be the target of
252-
// `simpleLocalFlowStep`.
253-
private predicate isPostUpdateNode(Node n) { n instanceof PostUpdateNode or none() }
254-
255-
query predicate postWithInFlow(Node n, string msg) {
256-
isPostUpdateNode(n) and
257-
not clearsContent(n, _) and
258-
simpleLocalFlowStep(_, n) and
259-
not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
260-
msg = "PostUpdateNode should not be the target of local flow."
261-
}
262-
263-
query predicate viableImplInCallContextTooLarge(
264-
DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
265-
) {
266-
callable = viableImplInCallContext(call, ctx) and
267-
not callable = viableCallable(call) and
268-
not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
43+
predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) {
44+
not exists(call.getLocation().getFile().getRelativePath())
26945
}
27046

271-
query predicate uniqueParameterNodeAtPosition(
272-
DataFlowCallable c, ParameterPosition pos, Node p, string msg
273-
) {
274-
not any(ConsistencyConfiguration conf).uniqueParameterNodeAtPositionExclude(c, pos, p) and
275-
isParameterNode(p, c, pos) and
276-
not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and
277-
msg = "Parameters with overlapping positions."
278-
}
279-
280-
query predicate uniqueParameterNodePosition(
281-
DataFlowCallable c, ParameterPosition pos, Node p, string msg
282-
) {
283-
not any(ConsistencyConfiguration conf).uniqueParameterNodePositionExclude(c, pos, p) and
284-
isParameterNode(p, c, pos) and
285-
not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and
286-
msg = "Parameter node with multiple positions."
287-
}
288-
289-
query predicate uniqueContentApprox(Content c, string msg) {
290-
not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and
291-
msg = "Non-unique content approximation."
292-
}
293-
294-
query predicate identityLocalStep(Node n, string msg) {
295-
simpleLocalFlowStep(n, n) and
296-
not any(ConsistencyConfiguration c).identityLocalStepExclude(n) and
297-
msg = "Node steps to itself"
47+
predicate identityLocalStepExclude(Node n) {
48+
not exists(n.getLocation().getFile().getRelativePath())
29849
}
29950
}
51+
52+
module Consistency = MakeConsistency<PythonDataFlow, PythonTaintTracking, Input>;

‎python/ql/test/experimental/dataflow/TestUtil/DataFlowConsistency.qll

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,6 @@
1+
// TODO: this should be promoted to be a REAL consistency query by being placed in
2+
// `python/ql/consistency-queries`. For for now it resides here.
13
import python
24
import semmle.python.dataflow.new.DataFlow::DataFlow
35
import semmle.python.dataflow.new.internal.DataFlowPrivate
46
import semmle.python.dataflow.new.internal.DataFlowImplConsistency::Consistency
5-
6-
// TODO: this should be promoted to be a REAL consistency query by being placed in
7-
// `python/ql/consistency-queries`. For for now it resides here.
8-
private class MyConsistencyConfiguration extends ConsistencyConfiguration {
9-
override predicate argHasPostUpdateExclude(ArgumentNode n) {
10-
exists(ArgumentPosition apos | n.argumentOf(_, apos) and apos.isStarArgs(_))
11-
or
12-
exists(ArgumentPosition apos | n.argumentOf(_, apos) and apos.isDictSplat())
13-
}
14-
15-
override predicate reverseReadExclude(Node n) {
16-
// since `self`/`cls` parameters can be marked as implicit argument to `super()`,
17-
// they will have PostUpdateNodes. We have a read-step from the synthetic `**kwargs`
18-
// parameter, but dataflow-consistency queries should _not_ complain about there not
19-
// being a post-update node for the synthetic `**kwargs` parameter.
20-
n instanceof SynthDictSplatParameterNode
21-
}
22-
23-
override predicate uniqueParameterNodePositionExclude(
24-
DataFlowCallable c, ParameterPosition pos, Node p
25-
) {
26-
// For normal parameters that can both be passed as positional arguments or keyword
27-
// arguments, we currently have parameter positions for both cases..
28-
//
29-
// TODO: Figure out how bad breaking this consistency check is
30-
exists(Function func, Parameter param |
31-
c.getScope() = func and
32-
p = parameterNode(param) and
33-
c.getParameter(pos) = p and
34-
param = func.getArg(_) and
35-
param = func.getArgByName(_)
36-
)
37-
}
38-
39-
override predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) {
40-
not exists(call.getLocation().getFile().getRelativePath())
41-
}
42-
43-
override predicate identityLocalStepExclude(Node n) {
44-
not exists(n.getLocation().getFile().getRelativePath())
45-
}
46-
}

0 commit comments

Comments
 (0)