Skip to content

Commit 58bf70d

Browse files
yoffyoff
committed
Python: filter self steps from use-use flow
Factor out use-use flow in order to do this. Also improve names and comments. I also wanted to change the types in `difinitionFlowStep`, but that broke the module instantiation.
1 parent 613831b commit 58bf70d

10 files changed

Lines changed: 51 additions & 84 deletions

File tree

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

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -279,14 +279,16 @@ class NonSyntheticPostUpdateNode extends PostUpdateNodeImpl, CfgNode {
279279
class DataFlowExpr = Expr;
280280

281281
/**
282-
* Flow between ESSA variables.
283-
* This includes both local and global variables.
284-
* Flow comes from definitions, uses and refinements.
282+
* A module to compute local flow.
283+
*
284+
* Flow will generally go from control flow nodes into essa variables at definitions,
285+
* and from there via use-use flow to other control flow nodes.
286+
*
287+
* Some syntaxtic constructs are handled separately.
285288
*/
286-
// TODO: Consider constraining `nodeFrom` and `nodeTo` to be in the same scope.
287-
// If they have different enclosing callables, we get consistency errors.
288-
module EssaFlow {
289-
predicate essaFlowStep(Node nodeFrom, Node nodeTo) {
289+
module LocalFlow {
290+
/** Holds if `nodeFrom` is the control flow node defining the essa variable `nodeTo`. */
291+
predicate definitionFlowStep(Node nodeFrom, Node nodeTo) {
290292
// Definition
291293
// `x = f(42)`
292294
// nodeFrom is `f(42)`, cfg node
@@ -336,24 +338,12 @@ module EssaFlow {
336338
// nodeFrom is `x`, cfgNode
337339
// nodeTo is `x`, essa var
338340
exists(ParameterDefinition pd |
339-
nodeFrom.asCfgNode() = pd.getDefiningNode() and
340-
nodeTo.asVar() = pd.getVariable()
341+
nodeFrom.(CfgNode).getNode() = pd.getDefiningNode() and
342+
nodeTo.(EssaNode).getVar() = pd.getVariable()
341343
)
342-
or
343-
// First use after definition
344-
// `y = 42`
345-
// `x = f(y)`
346-
// nodeFrom is `y` on first line, essa var
347-
// nodeTo is `y` on second line, cfg node
348-
defToFirstUse(nodeFrom.asVar(), nodeTo.asCfgNode())
349-
or
350-
// Next use after use
351-
// `x = f(y)`
352-
// `z = y + 1`
353-
// nodeFrom is 'y' on first line, cfg node
354-
// nodeTo is `y` on second line, cfg node
355-
useToNextUse(nodeFrom.asCfgNode(), nodeTo.asCfgNode())
356-
or
344+
}
345+
346+
predicate expressionFlowStep(Node nodeFrom, Node nodeTo) {
357347
// If expressions
358348
nodeFrom.asCfgNode() = nodeTo.asCfgNode().(IfExprNode).getAnOperand()
359349
or
@@ -366,6 +356,7 @@ module EssaFlow {
366356
// Flow inside an unpacking assignment
367357
iterableUnpackingFlowStep(nodeFrom, nodeTo)
368358
or
359+
// Flow inside a match statement
369360
matchFlowStep(nodeFrom, nodeTo)
370361
}
371362

@@ -376,6 +367,34 @@ module EssaFlow {
376367
predicate defToFirstUse(EssaVariable var, NameNode nodeTo) {
377368
AdjacentUses::firstUse(var.getDefinition(), nodeTo)
378369
}
370+
371+
predicate useUseFlowStep(Node nodeFrom, Node nodeTo) {
372+
// First use after definition
373+
// `y = 42`
374+
// `x = f(y)`
375+
// nodeFrom is `y` on first line, essa var
376+
// nodeTo is `y` on second line, cfg node
377+
defToFirstUse(nodeFrom.asVar(), nodeTo.asCfgNode())
378+
or
379+
// Next use after use
380+
// `x = f(y)`
381+
// `z = y + 1`
382+
// nodeFrom is 'y' on first line, cfg node
383+
// nodeTo is `y` on second line, cfg node
384+
useToNextUse(nodeFrom.asCfgNode(), nodeTo.asCfgNode())
385+
}
386+
387+
predicate localFlowStep(Node nodeFrom, Node nodeTo) {
388+
IncludePostUpdateFlow<PhaseDependentFlow<definitionFlowStep/2>::step/2>::step(nodeFrom, nodeTo)
389+
or
390+
IncludePostUpdateFlow<PhaseDependentFlow<expressionFlowStep/2>::step/2>::step(nodeFrom, nodeTo)
391+
or
392+
// Use-use flow can generate self loops. We want to filter steps from `n` to `n`
393+
// after we have included steps from `[post] n` to `n`, so after
394+
// `IncludePostUpdateFlow` has ben applied.
395+
IncludePostUpdateFlow<PhaseDependentFlow<useUseFlowStep/2>::step/2>::step(nodeFrom, nodeTo) and
396+
nodeFrom != nodeTo
397+
}
379398
}
380399

381400
//--------
@@ -481,7 +500,8 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
481500
* or at runtime when callables in the module are called.
482501
*/
483502
predicate simpleLocalFlowStepForTypetracking(Node nodeFrom, Node nodeTo) {
484-
IncludePostUpdateFlow<PhaseDependentFlow<EssaFlow::essaFlowStep/2>::step/2>::step(nodeFrom, nodeTo)
503+
IncludePostUpdateFlow<PhaseDependentFlow<LocalFlow::localFlowStep/2>::step/2>::step(nodeFrom,
504+
nodeTo)
485505
}
486506

487507
private predicate summaryLocalStep(Node nodeFrom, Node nodeTo) {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,13 @@ module ImportResolution {
111111
allowedEssaImportStep*(firstDef, lastUseVar) and
112112
not allowedEssaImportStep(_, firstDef)
113113
|
114-
not EssaFlow::defToFirstUse(firstDef, _) and
114+
not LocalFlow::defToFirstUse(firstDef, _) and
115115
val.asVar() = firstDef
116116
or
117117
exists(ControlFlowNode mid, ControlFlowNode end |
118-
EssaFlow::defToFirstUse(firstDef, mid) and
119-
EssaFlow::useToNextUse*(mid, end) and
120-
not EssaFlow::useToNextUse(end, _) and
118+
LocalFlow::defToFirstUse(firstDef, mid) and
119+
LocalFlow::useToNextUse*(mid, end) and
120+
not LocalFlow::useToNextUse(end, _) and
121121
val.asCfgNode() = end
122122
)
123123
)

‎python/ql/test/experimental/dataflow/coverage/dataflow-consistency.expected‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,5 @@ uniqueParameterNodeAtPosition
2525
uniqueParameterNodePosition
2626
uniqueContentApprox
2727
identityLocalStep
28-
| datamodel.py:84:15:84:15 | ControlFlowNode for x | Node steps to itself |
29-
| datamodel.py:166:11:166:11 | ControlFlowNode for x | Node steps to itself |
30-
| test.py:103:10:103:15 | ControlFlowNode for SOURCE | Node steps to itself |
31-
| test.py:130:10:130:15 | ControlFlowNode for SOURCE | Node steps to itself |
32-
| test.py:162:13:162:18 | ControlFlowNode for SOURCE | Node steps to itself |
33-
| test.py:167:13:167:18 | ControlFlowNode for SOURCE | Node steps to itself |
34-
| test.py:216:10:216:15 | ControlFlowNode for SOURCE | Node steps to itself |
35-
| test.py:242:9:242:12 | ControlFlowNode for SINK | Node steps to itself |
36-
| test.py:673:9:673:12 | ControlFlowNode for SINK | Node steps to itself |
37-
| test.py:674:9:674:14 | ControlFlowNode for SINK_F | Node steps to itself |
38-
| test.py:682:9:682:12 | ControlFlowNode for SINK | Node steps to itself |
39-
| test.py:690:9:690:12 | ControlFlowNode for SINK | Node steps to itself |
40-
| test.py:696:5:696:8 | ControlFlowNode for SINK | Node steps to itself |
4128
missingArgumentCall
4229
multipleArgumentCall

‎python/ql/test/experimental/dataflow/module-initialization/localFlow.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ module ImportTimeLocalFlowTest implements FlowTestSig {
1212
// results are displayed next to `nodeTo`, so we need a line to write on
1313
nodeTo.getLocation().getStartLine() > 0 and
1414
nodeTo.asVar() instanceof GlobalSsaVariable and
15-
DP::PhaseDependentFlow<DP::EssaFlow::essaFlowStep/2>::importTimeStep(nodeFrom, nodeTo)
15+
DP::PhaseDependentFlow<DP::LocalFlow::localFlowStep/2>::importTimeStep(nodeFrom, nodeTo)
1616
}
1717
}
1818

‎python/ql/test/experimental/dataflow/strange-essaflow/testFlow.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,5 @@ query predicate jumpStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
3333

3434
query predicate essaFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
3535
os_import(nodeFrom) and
36-
DataFlowPrivate::EssaFlow::essaFlowStep(nodeFrom, nodeTo)
36+
DataFlowPrivate::LocalFlow::localFlowStep(nodeFrom, nodeTo)
3737
}

‎python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep-py3/dataflow-consistency.expected‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,5 @@ uniqueParameterNodeAtPosition
2323
uniqueParameterNodePosition
2424
uniqueContentApprox
2525
identityLocalStep
26-
| test_collections.py:20:9:20:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
27-
| test_unpacking.py:31:9:31:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
2826
missingArgumentCall
2927
multipleArgumentCall

‎python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/dataflow-consistency.expected‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,5 @@ uniqueParameterNodeAtPosition
2323
uniqueParameterNodePosition
2424
uniqueContentApprox
2525
identityLocalStep
26-
| test_async.py:48:9:48:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
27-
| test_collections.py:64:10:64:21 | ControlFlowNode for tainted_list | Node steps to itself |
28-
| test_collections.py:71:9:71:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
29-
| test_collections.py:73:9:73:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
30-
| test_collections.py:88:10:88:21 | ControlFlowNode for tainted_list | Node steps to itself |
31-
| test_collections.py:89:10:89:23 | ControlFlowNode for TAINTED_STRING | Node steps to itself |
32-
| test_collections.py:97:9:97:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
33-
| test_collections.py:99:9:99:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
34-
| test_collections.py:112:9:112:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
35-
| test_collections.py:114:9:114:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
36-
| test_collections.py:147:9:147:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
37-
| test_collections.py:149:9:149:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
38-
| test_collections.py:246:9:246:15 | ControlFlowNode for my_dict | Node steps to itself |
39-
| test_collections.py:246:22:246:33 | ControlFlowNode for tainted_dict | Node steps to itself |
40-
| test_for.py:24:9:24:22 | ControlFlowNode for ensure_tainted | Node steps to itself |
4126
missingArgumentCall
4227
multipleArgumentCall

‎python/ql/test/experimental/dataflow/variable-capture/dataflow-consistency.expected‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,5 @@ uniqueParameterNodeAtPosition
2929
uniqueParameterNodePosition
3030
uniqueContentApprox
3131
identityLocalStep
32-
| test_collections.py:36:10:36:15 | ControlFlowNode for SOURCE | Node steps to itself |
33-
| test_collections.py:45:19:45:21 | ControlFlowNode for mod | Node steps to itself |
34-
| test_collections.py:52:13:52:21 | ControlFlowNode for mod_local | Node steps to itself |
3532
missingArgumentCall
3633
multipleArgumentCall

‎python/ql/test/library-tests/ApiGraphs/py3/dataflow-consistency.expected‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,5 @@ uniqueParameterNodeAtPosition
2727
uniqueParameterNodePosition
2828
uniqueContentApprox
2929
identityLocalStep
30-
| test_captured.py:7:22:7:22 | ControlFlowNode for p | Node steps to itself |
31-
| test_captured.py:14:26:14:27 | ControlFlowNode for pp | Node steps to itself |
3230
missingArgumentCall
3331
multipleArgumentCall

‎python/ql/test/library-tests/frameworks/django-orm/dataflow-consistency.expected‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -106,23 +106,5 @@ uniqueParameterNodeAtPosition
106106
uniqueParameterNodePosition
107107
uniqueContentApprox
108108
identityLocalStep
109-
| testapp/orm_tests.py:217:24:217:29 | ControlFlowNode for SOURCE | Node steps to itself |
110-
| testapp/orm_tests.py:244:24:244:29 | ControlFlowNode for SOURCE | Node steps to itself |
111-
| testapp/orm_tests.py:283:20:283:25 | ControlFlowNode for SOURCE | Node steps to itself |
112-
| testapp/orm_tests.py:299:15:299:22 | ControlFlowNode for TestLoad | Node steps to itself |
113-
| testapp/orm_tests.py:300:20:300:25 | ControlFlowNode for SOURCE | Node steps to itself |
114-
| testapp/orm_tests.py:310:9:310:12 | ControlFlowNode for SINK | Node steps to itself |
115-
| testapp/orm_tests.py:316:9:316:12 | ControlFlowNode for SINK | Node steps to itself |
116-
| testapp/orm_tests.py:326:9:326:12 | ControlFlowNode for SINK | Node steps to itself |
117-
| testapp/orm_tests.py:333:9:333:12 | ControlFlowNode for SINK | Node steps to itself |
118-
| testapp/orm_tests.py:339:9:339:12 | ControlFlowNode for SINK | Node steps to itself |
119-
| testapp/orm_tests.py:346:9:346:12 | ControlFlowNode for SINK | Node steps to itself |
120-
| testapp/orm_tests.py:352:9:352:12 | ControlFlowNode for SINK | Node steps to itself |
121-
| testapp/orm_tests.py:358:9:358:12 | ControlFlowNode for SINK | Node steps to itself |
122-
| testapp/orm_tests.py:365:9:365:12 | ControlFlowNode for SINK | Node steps to itself |
123-
| testapp/tests.py:12:13:12:14 | ControlFlowNode for re | Node steps to itself |
124-
| testapp/tests.py:16:9:16:18 | ControlFlowNode for test_names | Node steps to itself |
125-
| testapp/tests.py:25:13:25:14 | ControlFlowNode for re | Node steps to itself |
126-
| testapp/tests.py:31:9:31:18 | ControlFlowNode for test_names | Node steps to itself |
127109
missingArgumentCall
128110
multipleArgumentCall

0 commit comments

Comments
 (0)