From bea07002d380189afd900daa380eab8c27e6c624 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Thu, 6 Jul 2023 17:21:29 +0200 Subject: [PATCH 01/11] Python: Expand captured-variable test with default param --- .../test/experimental/dataflow/typetracking/test.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/python/ql/test/experimental/dataflow/typetracking/test.py b/python/ql/test/experimental/dataflow/typetracking/test.py index f0e93d79af2..25d82e912b3 100644 --- a/python/ql/test/experimental/dataflow/typetracking/test.py +++ b/python/ql/test/experimental/dataflow/typetracking/test.py @@ -65,6 +65,19 @@ def to_inner_scope(): also_x = foo() # $ tracked print(also_x) # $ tracked + +def from_parameter_default(): + x_alias = tracked # $tracked + def outer(x=tracked): # $tracked + print(x) # $tracked + def inner(): + print(x) # $ MISSING: tracked + print(x_alias) # $tracked + return x # $tracked + also_x = outer() # $tracked + print(also_x) # $tracked + + # ------------------------------------------------------------------------------ # Function decorator # ------------------------------------------------------------------------------ From cfd2d09a615a0f2e92e15b83d5b3397500ab57be Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 7 Jul 2023 09:54:56 +0200 Subject: [PATCH 02/11] Python: Add test for `DefinitionNode` default parameter value --- .../variables/definitions/definition-values.expected | 3 +++ .../library-tests/variables/definitions/definition-values.ql | 4 ++++ .../ql/test/library-tests/variables/definitions/test.expected | 3 ++- python/ql/test/library-tests/variables/definitions/test.py | 3 +++ 4 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 python/ql/test/library-tests/variables/definitions/definition-values.expected create mode 100644 python/ql/test/library-tests/variables/definitions/definition-values.ql diff --git a/python/ql/test/library-tests/variables/definitions/definition-values.expected b/python/ql/test/library-tests/variables/definitions/definition-values.expected new file mode 100644 index 00000000000..547f90f48cd --- /dev/null +++ b/python/ql/test/library-tests/variables/definitions/definition-values.expected @@ -0,0 +1,3 @@ +| test.py:3:5:3:9 | ControlFlowNode for fail5 | test.py:3:1:3:13 | ControlFlowNode for FunctionExpr | +| test.py:4:5:4:8 | ControlFlowNode for Tuple | test.py:4:12:4:12 | ControlFlowNode for t | +| test.py:7:5:7:26 | ControlFlowNode for default_value_in_param | test.py:7:1:7:33 | ControlFlowNode for FunctionExpr | diff --git a/python/ql/test/library-tests/variables/definitions/definition-values.ql b/python/ql/test/library-tests/variables/definitions/definition-values.ql new file mode 100644 index 00000000000..7c864541746 --- /dev/null +++ b/python/ql/test/library-tests/variables/definitions/definition-values.ql @@ -0,0 +1,4 @@ +import python + +from DefinitionNode d +select d, d.getValue() diff --git a/python/ql/test/library-tests/variables/definitions/test.expected b/python/ql/test/library-tests/variables/definitions/test.expected index dc853ee7f2f..dfcbbd236f1 100644 --- a/python/ql/test/library-tests/variables/definitions/test.expected +++ b/python/ql/test/library-tests/variables/definitions/test.expected @@ -1,4 +1,5 @@ | 3 | 5 | ControlFlowNode for fail5 | | 4 | 5 | ControlFlowNode for Tuple | | 4 | 5 | ControlFlowNode for x | -| 4 | 8 | ControlFlowNode for y | \ No newline at end of file +| 4 | 8 | ControlFlowNode for y | +| 7 | 5 | ControlFlowNode for default_value_in_param | diff --git a/python/ql/test/library-tests/variables/definitions/test.py b/python/ql/test/library-tests/variables/definitions/test.py index 0695a275b3c..c8cd2507399 100644 --- a/python/ql/test/library-tests/variables/definitions/test.py +++ b/python/ql/test/library-tests/variables/definitions/test.py @@ -3,3 +3,6 @@ def fail5(t): x, y = t return x + +def default_value_in_param(x=42): + print(x) From 6f3cb6705027497a6ee403d55c365a3614039269 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 7 Jul 2023 11:03:07 +0200 Subject: [PATCH 03/11] Python: Model parameter with default value as `DefinitionNode` --- python/ql/lib/semmle/python/Flow.qll | 15 ++++++++++++++- .../experimental/dataflow/typetracking/test.py | 2 +- .../definitions/definition-values.expected | 1 + .../variables/definitions/test.expected | 1 + 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/python/ql/lib/semmle/python/Flow.qll b/python/ql/lib/semmle/python/Flow.qll index 8dfdc4a6341..17a408c6713 100644 --- a/python/ql/lib/semmle/python/Flow.qll +++ b/python/ql/lib/semmle/python/Flow.qll @@ -640,12 +640,23 @@ class DefinitionNode extends ControlFlowNode { exists(Assign a | list_or_tuple_nested_element(a.getATarget()).getAFlowNode() = this) or exists(For for | for.getTarget().getAFlowNode() = this) + or + exists(Parameter param | this = param.asName().getAFlowNode() and exists(param.getDefault())) } /** flow node corresponding to the value assigned for the definition corresponding to this flow node */ ControlFlowNode getValue() { result = assigned_value(this.getNode()).getAFlowNode() and - (result.getBasicBlock().dominates(this.getBasicBlock()) or result.isImport()) + ( + result.getBasicBlock().dominates(this.getBasicBlock()) + or + result.isImport() + or + // since the default value for a parameter is evaluated in the same basic block as + // the function definition, but the parameter belongs to the basic block of the function, + // there is no dominance relationship between the two. + exists(Parameter param | this = param.asName().getAFlowNode()) + ) } } @@ -795,6 +806,8 @@ private AstNode assigned_value(Expr lhs) { or /* for lhs in seq: => `result` is the `for` node, representing the `iter(next(seq))` operation. */ result.(For).getTarget() = lhs + or + exists(Parameter param | lhs = param.asName() and result = param.getDefault()) } predicate nested_sequence_assign( diff --git a/python/ql/test/experimental/dataflow/typetracking/test.py b/python/ql/test/experimental/dataflow/typetracking/test.py index 25d82e912b3..a822ef683eb 100644 --- a/python/ql/test/experimental/dataflow/typetracking/test.py +++ b/python/ql/test/experimental/dataflow/typetracking/test.py @@ -71,7 +71,7 @@ def from_parameter_default(): def outer(x=tracked): # $tracked print(x) # $tracked def inner(): - print(x) # $ MISSING: tracked + print(x) # $ tracked print(x_alias) # $tracked return x # $tracked also_x = outer() # $tracked diff --git a/python/ql/test/library-tests/variables/definitions/definition-values.expected b/python/ql/test/library-tests/variables/definitions/definition-values.expected index 547f90f48cd..c81fafd6a64 100644 --- a/python/ql/test/library-tests/variables/definitions/definition-values.expected +++ b/python/ql/test/library-tests/variables/definitions/definition-values.expected @@ -1,3 +1,4 @@ | test.py:3:5:3:9 | ControlFlowNode for fail5 | test.py:3:1:3:13 | ControlFlowNode for FunctionExpr | | test.py:4:5:4:8 | ControlFlowNode for Tuple | test.py:4:12:4:12 | ControlFlowNode for t | | test.py:7:5:7:26 | ControlFlowNode for default_value_in_param | test.py:7:1:7:33 | ControlFlowNode for FunctionExpr | +| test.py:7:28:7:28 | ControlFlowNode for x | test.py:7:30:7:31 | ControlFlowNode for IntegerLiteral | diff --git a/python/ql/test/library-tests/variables/definitions/test.expected b/python/ql/test/library-tests/variables/definitions/test.expected index dfcbbd236f1..f5e119e6fb2 100644 --- a/python/ql/test/library-tests/variables/definitions/test.expected +++ b/python/ql/test/library-tests/variables/definitions/test.expected @@ -3,3 +3,4 @@ | 4 | 5 | ControlFlowNode for x | | 4 | 8 | ControlFlowNode for y | | 7 | 5 | ControlFlowNode for default_value_in_param | +| 7 | 28 | ControlFlowNode for x | From c5e8e232e52747b42c41d20ce3adeff59b868698 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 7 Jul 2023 11:55:07 +0200 Subject: [PATCH 04/11] Python: Fix dataflow consistencies for default parameter values --- .../dataflow/new/internal/DataFlowPrivate.qll | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll index 29504b6aa38..1569a62b2e9 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll @@ -292,7 +292,12 @@ module EssaFlow { // nodeFrom is `f(42)`, cfg node // nodeTo is `x`, essa var nodeFrom.(CfgNode).getNode() = - nodeTo.(EssaNode).getVar().getDefinition().(AssignmentDefinition).getValue() + nodeTo.(EssaNode).getVar().getDefinition().(AssignmentDefinition).getValue() and + // we need to ensure that enclosing callable is the same, since a parameter with a + // default value will be in the scope of the function, while the default value + // itself will be in the scope that _defines_ the function. + // We handle _that_ as a jumpstep + nodeFrom.getEnclosingCallable() = nodeTo.getEnclosingCallable() or // With definition // `with f(42) as x:` @@ -463,6 +468,13 @@ predicate runtimeJumpStep(Node nodeFrom, Node nodeTo) { or // Setting the possible values of the variable at the end of import time nodeFrom = nodeTo.(ModuleVariableNode).getADefiningWrite() + or + // a parameter with a default value, since the parameter will be in the scope of the + // function, while the default value itself will be in the scope that _defines_ the + // function. + nodeFrom.(CfgNode).getNode() = + nodeTo.(EssaNode).getVar().getDefinition().(AssignmentDefinition).getValue() and + not nodeFrom.getEnclosingCallable() = nodeTo.getEnclosingCallable() } /** From 70994b9c573c78169640a8cf9771dfa0eb51bc7c Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 7 Jul 2023 12:13:57 +0200 Subject: [PATCH 05/11] Python: Accept points-to .expected changes They look pretty safe to me, but haven't given them a whole lot of thought. --- .../PointsTo/new/Consistency.expected | 34 +++++++++++++++++++ .../PointsTo/new/Dataflow.expected | 18 +++++----- .../PointsTo/new/Definitions.expected | 5 +++ .../library-tests/PointsTo/new/SSA.expected | 20 +++++------ 4 files changed, 58 insertions(+), 19 deletions(-) diff --git a/python/ql/test/library-tests/PointsTo/new/Consistency.expected b/python/ql/test/library-tests/PointsTo/new/Consistency.expected index e69de29bb2d..194456bd452 100644 --- a/python/ql/test/library-tests/PointsTo/new/Consistency.expected +++ b/python/ql/test/library-tests/PointsTo/new/Consistency.expected @@ -0,0 +1,34 @@ +| AssignmentDefinition | a at code/m_attributes.py:5 | has non-disjoint subclasses | +| AssignmentDefinition | b at code/a_simple.py:38 | has non-disjoint subclasses | +| AssignmentDefinition | c at code/a_simple.py:38 | has non-disjoint subclasses | +| AssignmentDefinition | compile_ops at code/n_nesting.py:8 | has non-disjoint subclasses | +| AssignmentDefinition | file at /home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/_py_abc.py:72 | has non-disjoint subclasses | +| AssignmentDefinition | file at /home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/abc.py:104 | has non-disjoint subclasses | +| AssignmentDefinition | foo at code/b_condition.py:81 | has non-disjoint subclasses | +| AssignmentDefinition | name at code/r_regressions.py:58 | has non-disjoint subclasses | +| AssignmentDefinition | x at code/b_condition.py:87 | has non-disjoint subclasses | +| AssignmentDefinition | x at code/c_tests.py:71 | has non-disjoint subclasses | +| AssignmentDefinition | x at code/l_calls.py:3 | has non-disjoint subclasses | +| AssignmentDefinition | x at code/l_calls.py:6 | has non-disjoint subclasses | +| AssignmentDefinition | y at code/b_condition.py:87 | has non-disjoint subclasses | +| AssignmentDefinition | y at code/c_tests.py:71 | has non-disjoint subclasses | +| AssignmentDefinition | y at code/r_regressions.py:27 | has non-disjoint subclasses | +| AssignmentDefinition | z at code/l_calls.py:48 | has non-disjoint subclasses | +| AssignmentDefinition | z at code/r_regressions.py:27 | has non-disjoint subclasses | +| ParameterDefinition | a at code/m_attributes.py:5 | has non-disjoint subclasses | +| ParameterDefinition | b at code/a_simple.py:38 | has non-disjoint subclasses | +| ParameterDefinition | c at code/a_simple.py:38 | has non-disjoint subclasses | +| ParameterDefinition | compile_ops at code/n_nesting.py:8 | has non-disjoint subclasses | +| ParameterDefinition | file at /home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/_py_abc.py:72 | has non-disjoint subclasses | +| ParameterDefinition | file at /home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/abc.py:104 | has non-disjoint subclasses | +| ParameterDefinition | foo at code/b_condition.py:81 | has non-disjoint subclasses | +| ParameterDefinition | name at code/r_regressions.py:58 | has non-disjoint subclasses | +| ParameterDefinition | x at code/b_condition.py:87 | has non-disjoint subclasses | +| ParameterDefinition | x at code/c_tests.py:71 | has non-disjoint subclasses | +| ParameterDefinition | x at code/l_calls.py:3 | has non-disjoint subclasses | +| ParameterDefinition | x at code/l_calls.py:6 | has non-disjoint subclasses | +| ParameterDefinition | y at code/b_condition.py:87 | has non-disjoint subclasses | +| ParameterDefinition | y at code/c_tests.py:71 | has non-disjoint subclasses | +| ParameterDefinition | y at code/r_regressions.py:27 | has non-disjoint subclasses | +| ParameterDefinition | z at code/l_calls.py:48 | has non-disjoint subclasses | +| ParameterDefinition | z at code/r_regressions.py:27 | has non-disjoint subclasses | diff --git a/python/ql/test/library-tests/PointsTo/new/Dataflow.expected b/python/ql/test/library-tests/PointsTo/new/Dataflow.expected index aeee23bce72..abe94891d24 100644 --- a/python/ql/test/library-tests/PointsTo/new/Dataflow.expected +++ b/python/ql/test/library-tests/PointsTo/new/Dataflow.expected @@ -48,8 +48,8 @@ | a_simple.py:34 | f_0 = FunctionExpr | | a_simple.py:34 | kwargs_0 = ParameterDefinition | | a_simple.py:38 | a_0 = ParameterDefinition | -| a_simple.py:38 | b_0 = ParameterDefinition | -| a_simple.py:38 | c_0 = ParameterDefinition | +| a_simple.py:38 | b_0 = Str | +| a_simple.py:38 | c_0 = Str | | a_simple.py:38 | multi_assign_and_packing_0 = FunctionExpr | | a_simple.py:39 | t_0 = Tuple | | a_simple.py:40 | w_0 = Tuple | @@ -151,7 +151,7 @@ | b_condition.py:79 | t_4 = ArgumentRefinement(t_3) | | b_condition.py:81 | bar_0 = ScopeEntryDefinition | | b_condition.py:81 | bar_2 = phi(bar_0, bar_1) | -| b_condition.py:81 | foo_0 = ParameterDefinition | +| b_condition.py:81 | foo_0 = True | | b_condition.py:81 | foo_3 = Pi(foo_0) [false] | | b_condition.py:81 | foo_4 = phi(foo_1, foo_3) | | b_condition.py:81 | odasa6261_1 = FunctionExpr | @@ -159,8 +159,8 @@ | b_condition.py:83 | foo_1 = Pi(foo_0) [true] | | b_condition.py:83 | foo_2 = ScopeEntryDefinition | | b_condition.py:87 | split_bool1_1 = FunctionExpr | -| b_condition.py:87 | x_0 = ParameterDefinition | -| b_condition.py:87 | y_0 = ParameterDefinition | +| b_condition.py:87 | x_0 = None | +| b_condition.py:87 | y_0 = None | | b_condition.py:88 | x_1 = Pi(x_0) [true] | | b_condition.py:90 | x_4 = Pi(x_0) [false] | | b_condition.py:90 | x_5 = SingleSuccessorGuard(x_4) [false] | @@ -643,7 +643,7 @@ | n_nesting.py:0 | __name___0 = ScopeEntryDefinition | | n_nesting.py:0 | __package___0 = ScopeEntryDefinition | | n_nesting.py:8 | C_0 = ScopeEntryDefinition | -| n_nesting.py:8 | compile_ops_0 = ParameterDefinition | +| n_nesting.py:8 | compile_ops_0 = True | | n_nesting.py:8 | foo_0 = FunctionExpr | | n_nesting.py:9 | C_1 = CallsiteRefinement(C_0) | | n_nesting.py:10 | C_5 = ScopeEntryDefinition | @@ -722,10 +722,10 @@ | r_regressions.py:27 | gv_33 = phi(gv_12, gv_13) | | r_regressions.py:27 | x_0 = ParameterDefinition | | r_regressions.py:27 | x_5 = phi(x_3, x_4) | -| r_regressions.py:27 | y_0 = ParameterDefinition | +| r_regressions.py:27 | y_0 = None | | r_regressions.py:27 | y_7 = Pi(y_2) [false] | | r_regressions.py:27 | y_8 = phi(y_3, y_6, y_7) | -| r_regressions.py:27 | z_0 = ParameterDefinition | +| r_regressions.py:27 | z_0 = IntegerLiteral | | r_regressions.py:27 | z_3 = Pi(z_0) [false] | | r_regressions.py:27 | z_4 = phi(z_0, z_2, z_3) | | r_regressions.py:31 | x_1 = Pi(x_0) [true] | @@ -761,7 +761,7 @@ | r_regressions.py:58 | decorator_0 = ParameterDefinition | | r_regressions.py:58 | gv_19 = ScopeEntryDefinition | | r_regressions.py:58 | method_decorator_0 = FunctionExpr | -| r_regressions.py:58 | name_0 = ParameterDefinition | +| r_regressions.py:58 | name_0 = Str | | r_regressions.py:61 | _dec_0 = FunctionExpr | | r_regressions.py:61 | func_0 = ScopeEntryDefinition | | r_regressions.py:61 | gv_20 = ScopeEntryDefinition | diff --git a/python/ql/test/library-tests/PointsTo/new/Definitions.expected b/python/ql/test/library-tests/PointsTo/new/Definitions.expected index 7c7fd65c593..ce931319089 100644 --- a/python/ql/test/library-tests/PointsTo/new/Definitions.expected +++ b/python/ql/test/library-tests/PointsTo/new/Definitions.expected @@ -32,7 +32,9 @@ | a_simple.py:34 | Local Variable kwargs | ParameterDefinition | | a_simple.py:38 | Global Variable multi_assign_and_packing | AssignmentDefinition | | a_simple.py:38 | Local Variable a | ParameterDefinition | +| a_simple.py:38 | Local Variable b | AssignmentDefinition | | a_simple.py:38 | Local Variable b | ParameterDefinition | +| a_simple.py:38 | Local Variable c | AssignmentDefinition | | a_simple.py:38 | Local Variable c | ParameterDefinition | | a_simple.py:39 | Local Variable t | AssignmentDefinition | | a_simple.py:40 | Local Variable w | AssignmentDefinition | @@ -132,6 +134,7 @@ | b_condition.py:81 | Global Variable odasa6261 | AssignmentDefinition | | b_condition.py:81 | Local Variable bar | PhiFunction | | b_condition.py:81 | Local Variable bar | ScopeEntryDefinition | +| b_condition.py:81 | Local Variable foo | AssignmentDefinition | | b_condition.py:81 | Local Variable foo | ParameterDefinition | | b_condition.py:81 | Local Variable foo | PhiFunction | | b_condition.py:81 | Local Variable foo | PyEdgeRefinement | @@ -139,7 +142,9 @@ | b_condition.py:83 | Local Variable foo | PyEdgeRefinement | | b_condition.py:83 | Local Variable foo | ScopeEntryDefinition | | b_condition.py:87 | Global Variable split_bool1 | AssignmentDefinition | +| b_condition.py:87 | Local Variable x | AssignmentDefinition | | b_condition.py:87 | Local Variable x | ParameterDefinition | +| b_condition.py:87 | Local Variable y | AssignmentDefinition | | b_condition.py:87 | Local Variable y | ParameterDefinition | | b_condition.py:88 | Local Variable x | PyEdgeRefinement | | b_condition.py:90 | Local Variable x | PyEdgeRefinement | diff --git a/python/ql/test/library-tests/PointsTo/new/SSA.expected b/python/ql/test/library-tests/PointsTo/new/SSA.expected index 9914a4eeec7..e6ed2435dd2 100644 --- a/python/ql/test/library-tests/PointsTo/new/SSA.expected +++ b/python/ql/test/library-tests/PointsTo/new/SSA.expected @@ -25,8 +25,8 @@ | a_simple.py:23 | with_definition_0 = FunctionExpr | Function with_definition | builtin-class function | | a_simple.py:27 | multi_loop_in_try_0 = FunctionExpr | Function multi_loop_in_try | builtin-class function | | a_simple.py:34 | f_0 = FunctionExpr | Function f | builtin-class function | -| a_simple.py:38 | b_0 = ParameterDefinition | 'b' | builtin-class str | -| a_simple.py:38 | c_0 = ParameterDefinition | 'c' | builtin-class str | +| a_simple.py:38 | b_0 = Str | 'b' | builtin-class str | +| a_simple.py:38 | c_0 = Str | 'c' | builtin-class str | | a_simple.py:38 | multi_assign_and_packing_0 = FunctionExpr | Function multi_assign_and_packing | builtin-class function | | a_simple.py:39 | t_0 = Tuple | Tuple | builtin-class tuple | | a_simple.py:40 | w_0 = Tuple | Tuple | builtin-class tuple | @@ -81,14 +81,14 @@ | b_condition.py:79 | t_3 = phi(t_1, t_2) | builtin-class object | builtin-class type | | b_condition.py:79 | t_4 = ArgumentRefinement(t_3) | builtin-class object | builtin-class type | | b_condition.py:81 | bar_2 = phi(bar_0, bar_1) | Function bar | builtin-class function | -| b_condition.py:81 | foo_0 = ParameterDefinition | bool True | builtin-class bool | +| b_condition.py:81 | foo_0 = True | bool True | builtin-class bool | | b_condition.py:81 | foo_3 = Pi(foo_0) [false] | bool True | builtin-class bool | | b_condition.py:81 | foo_4 = phi(foo_1, foo_3) | bool True | builtin-class bool | | b_condition.py:81 | odasa6261_1 = FunctionExpr | Function odasa6261 | builtin-class function | | b_condition.py:83 | bar_1 = FunctionExpr | Function bar | builtin-class function | | b_condition.py:87 | split_bool1_1 = FunctionExpr | Function split_bool1 | builtin-class function | -| b_condition.py:87 | x_0 = ParameterDefinition | NoneType None | builtin-class NoneType | -| b_condition.py:87 | y_0 = ParameterDefinition | NoneType None | builtin-class NoneType | +| b_condition.py:87 | x_0 = None | NoneType None | builtin-class NoneType | +| b_condition.py:87 | y_0 = None | NoneType None | builtin-class NoneType | | b_condition.py:90 | x_4 = Pi(x_0) [false] | NoneType None | builtin-class NoneType | | b_condition.py:90 | x_5 = SingleSuccessorGuard(x_4) [false] | NoneType None | builtin-class NoneType | | b_condition.py:90 | y_4 = Pi(y_0) [false] | NoneType None | builtin-class NoneType | @@ -140,8 +140,8 @@ | c_tests.py:68 | x_5 = phi(x_3, x_4) | int 0 | builtin-class int | | c_tests.py:69 | x_6 = Pi(x_5) [true] | builtin-class float | builtin-class type | | c_tests.py:71 | compound_0 = FunctionExpr | Function compound | builtin-class function | -| c_tests.py:71 | x_0 = ParameterDefinition | int 1 | builtin-class int | -| c_tests.py:71 | y_0 = ParameterDefinition | int 0 | builtin-class int | +| c_tests.py:71 | x_0 = IntegerLiteral | int 1 | builtin-class int | +| c_tests.py:71 | y_0 = IntegerLiteral | int 0 | builtin-class int | | c_tests.py:71 | y_5 = Pi(y_0) [false] | int 0 | builtin-class int | | c_tests.py:71 | y_6 = phi(y_4, y_5) | int 0 | builtin-class int | | c_tests.py:74 | x_2 = Pi(x_0) [true] | int 1 | builtin-class int | @@ -387,8 +387,8 @@ | m_attributes.py:0 | __name___0 = ScopeEntryDefinition | 'code.m_attributes' | builtin-class str | | m_attributes.py:3 | C_0 = ClassExpr | class C | builtin-class type | | m_attributes.py:5 | __init___0 = FunctionExpr | Function __init__ | builtin-class function | -| m_attributes.py:5 | a_0 = ParameterDefinition | int 17 | builtin-class int | -| m_attributes.py:5 | a_0 = ParameterDefinition | int 100 | builtin-class int | +| m_attributes.py:5 | a_0 = IntegerLiteral | int 17 | builtin-class int | +| m_attributes.py:5 | a_0 = IntegerLiteral | int 100 | builtin-class int | | m_attributes.py:5 | self_0 = ParameterDefinition | self | class C | | m_attributes.py:6 | self_1 = AttributeAssignment 'a'(self_0) | self | class C | | m_attributes.py:8 | foo_0 = FunctionExpr | Function foo | builtin-class function | @@ -397,7 +397,7 @@ | m_attributes.py:8 | self_0 = ParameterDefinition | self | class C | | n_nesting.py:0 | __name___0 = ScopeEntryDefinition | 'code.n_nesting' | builtin-class str | | n_nesting.py:8 | C_0 = ScopeEntryDefinition | int 1 | builtin-class int | -| n_nesting.py:8 | compile_ops_0 = ParameterDefinition | bool True | builtin-class bool | +| n_nesting.py:8 | compile_ops_0 = True | bool True | builtin-class bool | | n_nesting.py:8 | foo_0 = FunctionExpr | Function foo | builtin-class function | | n_nesting.py:9 | C_1 = CallsiteRefinement(C_0) | int 1 | builtin-class int | | n_nesting.py:10 | C_5 = ScopeEntryDefinition | int 1 | builtin-class int | From 4e8a1144f214901cb6aae55d71d3d4209d1c9122 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 7 Jul 2023 14:09:30 +0200 Subject: [PATCH 06/11] Python: Remove explicit jumpStep for default parameter values tests added in https://github.com/github/codeql/pull/5238 functionality added in https://github.com/github/codeql/pull/6640 --- .../dataflow/new/internal/DataFlowPrivate.qll | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll index 1569a62b2e9..d6d8d0c1297 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll @@ -576,9 +576,6 @@ predicate jumpStepSharedWithTypeTracker(Node nodeFrom, Node nodeTo) { r.getAttributeName(), nodeFrom) and nodeTo = r ) - or - // Default value for parameter flows to that parameter - defaultValueFlowStep(nodeFrom, nodeTo) } /** @@ -799,19 +796,6 @@ predicate attributeStoreStep(Node nodeFrom, AttributeContent c, PostUpdateNode n ) } -predicate defaultValueFlowStep(CfgNode nodeFrom, CfgNode nodeTo) { - exists(Function f, Parameter p, ParameterDefinition def | - // `getArgByName` supports, unlike `getAnArg`, keyword-only parameters - p = f.getArgByName(_) and - nodeFrom.asExpr() = p.getDefault() and - // The following expresses - // nodeTo.(ParameterNode).getParameter() = p - // without non-monotonic recursion - def.getParameter() = p and - nodeTo.getNode() = def.getDefiningNode() - ) -} - /** * Holds if data can flow from `nodeFrom` to `nodeTo` via a read of content `c`. */ From 43b025015d1612ae18bbd93b0b73e19ad1a9112a Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 7 Jul 2023 14:26:28 +0200 Subject: [PATCH 07/11] Python: Avoid overlap between `AssignmentDefinition` and `ParameterDefinition` --- .../dataflow/new/internal/DataFlowPrivate.qll | 10 ++---- python/ql/lib/semmle/python/essa/Essa.qll | 3 +- .../PointsTo/new/Consistency.expected | 34 ------------------- .../PointsTo/new/Dataflow.expected | 18 +++++----- .../PointsTo/new/Definitions.expected | 5 --- .../PointsTo/new/ImpliesDataflow.expected | 2 ++ .../library-tests/PointsTo/new/SSA.expected | 20 +++++------ 7 files changed, 25 insertions(+), 67 deletions(-) diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll index d6d8d0c1297..eafc16db734 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll @@ -292,12 +292,7 @@ module EssaFlow { // nodeFrom is `f(42)`, cfg node // nodeTo is `x`, essa var nodeFrom.(CfgNode).getNode() = - nodeTo.(EssaNode).getVar().getDefinition().(AssignmentDefinition).getValue() and - // we need to ensure that enclosing callable is the same, since a parameter with a - // default value will be in the scope of the function, while the default value - // itself will be in the scope that _defines_ the function. - // We handle _that_ as a jumpstep - nodeFrom.getEnclosingCallable() = nodeTo.getEnclosingCallable() + nodeTo.(EssaNode).getVar().getDefinition().(AssignmentDefinition).getValue() or // With definition // `with f(42) as x:` @@ -473,8 +468,7 @@ predicate runtimeJumpStep(Node nodeFrom, Node nodeTo) { // function, while the default value itself will be in the scope that _defines_ the // function. nodeFrom.(CfgNode).getNode() = - nodeTo.(EssaNode).getVar().getDefinition().(AssignmentDefinition).getValue() and - not nodeFrom.getEnclosingCallable() = nodeTo.getEnclosingCallable() + nodeTo.(EssaNode).getVar().getDefinition().(ParameterDefinition).getDefault() } /** diff --git a/python/ql/lib/semmle/python/essa/Essa.qll b/python/ql/lib/semmle/python/essa/Essa.qll index cf2aca1e2ac..6e9d328f7d5 100644 --- a/python/ql/lib/semmle/python/essa/Essa.qll +++ b/python/ql/lib/semmle/python/essa/Essa.qll @@ -501,7 +501,8 @@ class AssignmentDefinition extends EssaNodeDefinition { ControlFlowNode value; AssignmentDefinition() { - SsaSource::assignment_definition(this.getSourceVariable(), this.getDefiningNode(), value) + SsaSource::assignment_definition(this.getSourceVariable(), this.getDefiningNode(), value) and + not this instanceof ParameterDefinition } ControlFlowNode getValue() { result = value } diff --git a/python/ql/test/library-tests/PointsTo/new/Consistency.expected b/python/ql/test/library-tests/PointsTo/new/Consistency.expected index 194456bd452..e69de29bb2d 100644 --- a/python/ql/test/library-tests/PointsTo/new/Consistency.expected +++ b/python/ql/test/library-tests/PointsTo/new/Consistency.expected @@ -1,34 +0,0 @@ -| AssignmentDefinition | a at code/m_attributes.py:5 | has non-disjoint subclasses | -| AssignmentDefinition | b at code/a_simple.py:38 | has non-disjoint subclasses | -| AssignmentDefinition | c at code/a_simple.py:38 | has non-disjoint subclasses | -| AssignmentDefinition | compile_ops at code/n_nesting.py:8 | has non-disjoint subclasses | -| AssignmentDefinition | file at /home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/_py_abc.py:72 | has non-disjoint subclasses | -| AssignmentDefinition | file at /home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/abc.py:104 | has non-disjoint subclasses | -| AssignmentDefinition | foo at code/b_condition.py:81 | has non-disjoint subclasses | -| AssignmentDefinition | name at code/r_regressions.py:58 | has non-disjoint subclasses | -| AssignmentDefinition | x at code/b_condition.py:87 | has non-disjoint subclasses | -| AssignmentDefinition | x at code/c_tests.py:71 | has non-disjoint subclasses | -| AssignmentDefinition | x at code/l_calls.py:3 | has non-disjoint subclasses | -| AssignmentDefinition | x at code/l_calls.py:6 | has non-disjoint subclasses | -| AssignmentDefinition | y at code/b_condition.py:87 | has non-disjoint subclasses | -| AssignmentDefinition | y at code/c_tests.py:71 | has non-disjoint subclasses | -| AssignmentDefinition | y at code/r_regressions.py:27 | has non-disjoint subclasses | -| AssignmentDefinition | z at code/l_calls.py:48 | has non-disjoint subclasses | -| AssignmentDefinition | z at code/r_regressions.py:27 | has non-disjoint subclasses | -| ParameterDefinition | a at code/m_attributes.py:5 | has non-disjoint subclasses | -| ParameterDefinition | b at code/a_simple.py:38 | has non-disjoint subclasses | -| ParameterDefinition | c at code/a_simple.py:38 | has non-disjoint subclasses | -| ParameterDefinition | compile_ops at code/n_nesting.py:8 | has non-disjoint subclasses | -| ParameterDefinition | file at /home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/_py_abc.py:72 | has non-disjoint subclasses | -| ParameterDefinition | file at /home/rasmus/.pyenv/versions/3.9.5/lib/python3.9/abc.py:104 | has non-disjoint subclasses | -| ParameterDefinition | foo at code/b_condition.py:81 | has non-disjoint subclasses | -| ParameterDefinition | name at code/r_regressions.py:58 | has non-disjoint subclasses | -| ParameterDefinition | x at code/b_condition.py:87 | has non-disjoint subclasses | -| ParameterDefinition | x at code/c_tests.py:71 | has non-disjoint subclasses | -| ParameterDefinition | x at code/l_calls.py:3 | has non-disjoint subclasses | -| ParameterDefinition | x at code/l_calls.py:6 | has non-disjoint subclasses | -| ParameterDefinition | y at code/b_condition.py:87 | has non-disjoint subclasses | -| ParameterDefinition | y at code/c_tests.py:71 | has non-disjoint subclasses | -| ParameterDefinition | y at code/r_regressions.py:27 | has non-disjoint subclasses | -| ParameterDefinition | z at code/l_calls.py:48 | has non-disjoint subclasses | -| ParameterDefinition | z at code/r_regressions.py:27 | has non-disjoint subclasses | diff --git a/python/ql/test/library-tests/PointsTo/new/Dataflow.expected b/python/ql/test/library-tests/PointsTo/new/Dataflow.expected index abe94891d24..aeee23bce72 100644 --- a/python/ql/test/library-tests/PointsTo/new/Dataflow.expected +++ b/python/ql/test/library-tests/PointsTo/new/Dataflow.expected @@ -48,8 +48,8 @@ | a_simple.py:34 | f_0 = FunctionExpr | | a_simple.py:34 | kwargs_0 = ParameterDefinition | | a_simple.py:38 | a_0 = ParameterDefinition | -| a_simple.py:38 | b_0 = Str | -| a_simple.py:38 | c_0 = Str | +| a_simple.py:38 | b_0 = ParameterDefinition | +| a_simple.py:38 | c_0 = ParameterDefinition | | a_simple.py:38 | multi_assign_and_packing_0 = FunctionExpr | | a_simple.py:39 | t_0 = Tuple | | a_simple.py:40 | w_0 = Tuple | @@ -151,7 +151,7 @@ | b_condition.py:79 | t_4 = ArgumentRefinement(t_3) | | b_condition.py:81 | bar_0 = ScopeEntryDefinition | | b_condition.py:81 | bar_2 = phi(bar_0, bar_1) | -| b_condition.py:81 | foo_0 = True | +| b_condition.py:81 | foo_0 = ParameterDefinition | | b_condition.py:81 | foo_3 = Pi(foo_0) [false] | | b_condition.py:81 | foo_4 = phi(foo_1, foo_3) | | b_condition.py:81 | odasa6261_1 = FunctionExpr | @@ -159,8 +159,8 @@ | b_condition.py:83 | foo_1 = Pi(foo_0) [true] | | b_condition.py:83 | foo_2 = ScopeEntryDefinition | | b_condition.py:87 | split_bool1_1 = FunctionExpr | -| b_condition.py:87 | x_0 = None | -| b_condition.py:87 | y_0 = None | +| b_condition.py:87 | x_0 = ParameterDefinition | +| b_condition.py:87 | y_0 = ParameterDefinition | | b_condition.py:88 | x_1 = Pi(x_0) [true] | | b_condition.py:90 | x_4 = Pi(x_0) [false] | | b_condition.py:90 | x_5 = SingleSuccessorGuard(x_4) [false] | @@ -643,7 +643,7 @@ | n_nesting.py:0 | __name___0 = ScopeEntryDefinition | | n_nesting.py:0 | __package___0 = ScopeEntryDefinition | | n_nesting.py:8 | C_0 = ScopeEntryDefinition | -| n_nesting.py:8 | compile_ops_0 = True | +| n_nesting.py:8 | compile_ops_0 = ParameterDefinition | | n_nesting.py:8 | foo_0 = FunctionExpr | | n_nesting.py:9 | C_1 = CallsiteRefinement(C_0) | | n_nesting.py:10 | C_5 = ScopeEntryDefinition | @@ -722,10 +722,10 @@ | r_regressions.py:27 | gv_33 = phi(gv_12, gv_13) | | r_regressions.py:27 | x_0 = ParameterDefinition | | r_regressions.py:27 | x_5 = phi(x_3, x_4) | -| r_regressions.py:27 | y_0 = None | +| r_regressions.py:27 | y_0 = ParameterDefinition | | r_regressions.py:27 | y_7 = Pi(y_2) [false] | | r_regressions.py:27 | y_8 = phi(y_3, y_6, y_7) | -| r_regressions.py:27 | z_0 = IntegerLiteral | +| r_regressions.py:27 | z_0 = ParameterDefinition | | r_regressions.py:27 | z_3 = Pi(z_0) [false] | | r_regressions.py:27 | z_4 = phi(z_0, z_2, z_3) | | r_regressions.py:31 | x_1 = Pi(x_0) [true] | @@ -761,7 +761,7 @@ | r_regressions.py:58 | decorator_0 = ParameterDefinition | | r_regressions.py:58 | gv_19 = ScopeEntryDefinition | | r_regressions.py:58 | method_decorator_0 = FunctionExpr | -| r_regressions.py:58 | name_0 = Str | +| r_regressions.py:58 | name_0 = ParameterDefinition | | r_regressions.py:61 | _dec_0 = FunctionExpr | | r_regressions.py:61 | func_0 = ScopeEntryDefinition | | r_regressions.py:61 | gv_20 = ScopeEntryDefinition | diff --git a/python/ql/test/library-tests/PointsTo/new/Definitions.expected b/python/ql/test/library-tests/PointsTo/new/Definitions.expected index ce931319089..7c7fd65c593 100644 --- a/python/ql/test/library-tests/PointsTo/new/Definitions.expected +++ b/python/ql/test/library-tests/PointsTo/new/Definitions.expected @@ -32,9 +32,7 @@ | a_simple.py:34 | Local Variable kwargs | ParameterDefinition | | a_simple.py:38 | Global Variable multi_assign_and_packing | AssignmentDefinition | | a_simple.py:38 | Local Variable a | ParameterDefinition | -| a_simple.py:38 | Local Variable b | AssignmentDefinition | | a_simple.py:38 | Local Variable b | ParameterDefinition | -| a_simple.py:38 | Local Variable c | AssignmentDefinition | | a_simple.py:38 | Local Variable c | ParameterDefinition | | a_simple.py:39 | Local Variable t | AssignmentDefinition | | a_simple.py:40 | Local Variable w | AssignmentDefinition | @@ -134,7 +132,6 @@ | b_condition.py:81 | Global Variable odasa6261 | AssignmentDefinition | | b_condition.py:81 | Local Variable bar | PhiFunction | | b_condition.py:81 | Local Variable bar | ScopeEntryDefinition | -| b_condition.py:81 | Local Variable foo | AssignmentDefinition | | b_condition.py:81 | Local Variable foo | ParameterDefinition | | b_condition.py:81 | Local Variable foo | PhiFunction | | b_condition.py:81 | Local Variable foo | PyEdgeRefinement | @@ -142,9 +139,7 @@ | b_condition.py:83 | Local Variable foo | PyEdgeRefinement | | b_condition.py:83 | Local Variable foo | ScopeEntryDefinition | | b_condition.py:87 | Global Variable split_bool1 | AssignmentDefinition | -| b_condition.py:87 | Local Variable x | AssignmentDefinition | | b_condition.py:87 | Local Variable x | ParameterDefinition | -| b_condition.py:87 | Local Variable y | AssignmentDefinition | | b_condition.py:87 | Local Variable y | ParameterDefinition | | b_condition.py:88 | Local Variable x | PyEdgeRefinement | | b_condition.py:90 | Local Variable x | PyEdgeRefinement | diff --git a/python/ql/test/library-tests/PointsTo/new/ImpliesDataflow.expected b/python/ql/test/library-tests/PointsTo/new/ImpliesDataflow.expected index 0c2bd1b4ce0..171f45410b7 100644 --- a/python/ql/test/library-tests/PointsTo/new/ImpliesDataflow.expected +++ b/python/ql/test/library-tests/PointsTo/new/ImpliesDataflow.expected @@ -1,5 +1,7 @@ | code/h_classes.py:3:1:3:16 | ControlFlowNode for ClassExpr | code/h_classes.py:10:1:10:9 | ControlFlowNode for type() | | code/h_classes.py:3:1:3:16 | ControlFlowNode for ClassExpr | code/h_classes.py:15:5:15:13 | ControlFlowNode for type() | +| code/l_calls.py:3:13:3:14 | ControlFlowNode for List | code/l_calls.py:4:12:4:12 | ControlFlowNode for x | +| code/l_calls.py:6:13:6:14 | ControlFlowNode for List | code/l_calls.py:7:16:7:16 | ControlFlowNode for x | | code/l_calls.py:12:1:12:20 | ControlFlowNode for ClassExpr | code/l_calls.py:16:16:16:18 | ControlFlowNode for cls | | code/l_calls.py:12:1:12:20 | ControlFlowNode for ClassExpr | code/l_calls.py:24:13:24:22 | ControlFlowNode for Attribute() | | code/l_calls.py:12:1:12:20 | ControlFlowNode for ClassExpr | code/l_calls.py:25:16:25:16 | ControlFlowNode for a | diff --git a/python/ql/test/library-tests/PointsTo/new/SSA.expected b/python/ql/test/library-tests/PointsTo/new/SSA.expected index e6ed2435dd2..9914a4eeec7 100644 --- a/python/ql/test/library-tests/PointsTo/new/SSA.expected +++ b/python/ql/test/library-tests/PointsTo/new/SSA.expected @@ -25,8 +25,8 @@ | a_simple.py:23 | with_definition_0 = FunctionExpr | Function with_definition | builtin-class function | | a_simple.py:27 | multi_loop_in_try_0 = FunctionExpr | Function multi_loop_in_try | builtin-class function | | a_simple.py:34 | f_0 = FunctionExpr | Function f | builtin-class function | -| a_simple.py:38 | b_0 = Str | 'b' | builtin-class str | -| a_simple.py:38 | c_0 = Str | 'c' | builtin-class str | +| a_simple.py:38 | b_0 = ParameterDefinition | 'b' | builtin-class str | +| a_simple.py:38 | c_0 = ParameterDefinition | 'c' | builtin-class str | | a_simple.py:38 | multi_assign_and_packing_0 = FunctionExpr | Function multi_assign_and_packing | builtin-class function | | a_simple.py:39 | t_0 = Tuple | Tuple | builtin-class tuple | | a_simple.py:40 | w_0 = Tuple | Tuple | builtin-class tuple | @@ -81,14 +81,14 @@ | b_condition.py:79 | t_3 = phi(t_1, t_2) | builtin-class object | builtin-class type | | b_condition.py:79 | t_4 = ArgumentRefinement(t_3) | builtin-class object | builtin-class type | | b_condition.py:81 | bar_2 = phi(bar_0, bar_1) | Function bar | builtin-class function | -| b_condition.py:81 | foo_0 = True | bool True | builtin-class bool | +| b_condition.py:81 | foo_0 = ParameterDefinition | bool True | builtin-class bool | | b_condition.py:81 | foo_3 = Pi(foo_0) [false] | bool True | builtin-class bool | | b_condition.py:81 | foo_4 = phi(foo_1, foo_3) | bool True | builtin-class bool | | b_condition.py:81 | odasa6261_1 = FunctionExpr | Function odasa6261 | builtin-class function | | b_condition.py:83 | bar_1 = FunctionExpr | Function bar | builtin-class function | | b_condition.py:87 | split_bool1_1 = FunctionExpr | Function split_bool1 | builtin-class function | -| b_condition.py:87 | x_0 = None | NoneType None | builtin-class NoneType | -| b_condition.py:87 | y_0 = None | NoneType None | builtin-class NoneType | +| b_condition.py:87 | x_0 = ParameterDefinition | NoneType None | builtin-class NoneType | +| b_condition.py:87 | y_0 = ParameterDefinition | NoneType None | builtin-class NoneType | | b_condition.py:90 | x_4 = Pi(x_0) [false] | NoneType None | builtin-class NoneType | | b_condition.py:90 | x_5 = SingleSuccessorGuard(x_4) [false] | NoneType None | builtin-class NoneType | | b_condition.py:90 | y_4 = Pi(y_0) [false] | NoneType None | builtin-class NoneType | @@ -140,8 +140,8 @@ | c_tests.py:68 | x_5 = phi(x_3, x_4) | int 0 | builtin-class int | | c_tests.py:69 | x_6 = Pi(x_5) [true] | builtin-class float | builtin-class type | | c_tests.py:71 | compound_0 = FunctionExpr | Function compound | builtin-class function | -| c_tests.py:71 | x_0 = IntegerLiteral | int 1 | builtin-class int | -| c_tests.py:71 | y_0 = IntegerLiteral | int 0 | builtin-class int | +| c_tests.py:71 | x_0 = ParameterDefinition | int 1 | builtin-class int | +| c_tests.py:71 | y_0 = ParameterDefinition | int 0 | builtin-class int | | c_tests.py:71 | y_5 = Pi(y_0) [false] | int 0 | builtin-class int | | c_tests.py:71 | y_6 = phi(y_4, y_5) | int 0 | builtin-class int | | c_tests.py:74 | x_2 = Pi(x_0) [true] | int 1 | builtin-class int | @@ -387,8 +387,8 @@ | m_attributes.py:0 | __name___0 = ScopeEntryDefinition | 'code.m_attributes' | builtin-class str | | m_attributes.py:3 | C_0 = ClassExpr | class C | builtin-class type | | m_attributes.py:5 | __init___0 = FunctionExpr | Function __init__ | builtin-class function | -| m_attributes.py:5 | a_0 = IntegerLiteral | int 17 | builtin-class int | -| m_attributes.py:5 | a_0 = IntegerLiteral | int 100 | builtin-class int | +| m_attributes.py:5 | a_0 = ParameterDefinition | int 17 | builtin-class int | +| m_attributes.py:5 | a_0 = ParameterDefinition | int 100 | builtin-class int | | m_attributes.py:5 | self_0 = ParameterDefinition | self | class C | | m_attributes.py:6 | self_1 = AttributeAssignment 'a'(self_0) | self | class C | | m_attributes.py:8 | foo_0 = FunctionExpr | Function foo | builtin-class function | @@ -397,7 +397,7 @@ | m_attributes.py:8 | self_0 = ParameterDefinition | self | class C | | n_nesting.py:0 | __name___0 = ScopeEntryDefinition | 'code.n_nesting' | builtin-class str | | n_nesting.py:8 | C_0 = ScopeEntryDefinition | int 1 | builtin-class int | -| n_nesting.py:8 | compile_ops_0 = True | bool True | builtin-class bool | +| n_nesting.py:8 | compile_ops_0 = ParameterDefinition | bool True | builtin-class bool | | n_nesting.py:8 | foo_0 = FunctionExpr | Function foo | builtin-class function | | n_nesting.py:9 | C_1 = CallsiteRefinement(C_0) | int 1 | builtin-class int | | n_nesting.py:10 | C_5 = ScopeEntryDefinition | int 1 | builtin-class int | From 44c67171f21b877c0fff37ee7dc8b4e0c134cef7 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Fri, 7 Jul 2023 16:05:27 +0200 Subject: [PATCH 08/11] Python: Fix default parameter value flow Somehow the previous fix didn't work :O --- .../semmle/python/dataflow/new/internal/DataFlowPrivate.qll | 6 ++++-- .../library-tests/PointsTo/new/ImpliesDataflow.expected | 2 -- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll index eafc16db734..060af09edc7 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll @@ -467,8 +467,10 @@ predicate runtimeJumpStep(Node nodeFrom, Node nodeTo) { // a parameter with a default value, since the parameter will be in the scope of the // function, while the default value itself will be in the scope that _defines_ the // function. - nodeFrom.(CfgNode).getNode() = - nodeTo.(EssaNode).getVar().getDefinition().(ParameterDefinition).getDefault() + exists(ParameterDefinition param | + nodeFrom.asCfgNode() = param.getDefault() and + nodeTo.asCfgNode() = param.getDefiningNode() + ) } /** diff --git a/python/ql/test/library-tests/PointsTo/new/ImpliesDataflow.expected b/python/ql/test/library-tests/PointsTo/new/ImpliesDataflow.expected index 171f45410b7..0c2bd1b4ce0 100644 --- a/python/ql/test/library-tests/PointsTo/new/ImpliesDataflow.expected +++ b/python/ql/test/library-tests/PointsTo/new/ImpliesDataflow.expected @@ -1,7 +1,5 @@ | code/h_classes.py:3:1:3:16 | ControlFlowNode for ClassExpr | code/h_classes.py:10:1:10:9 | ControlFlowNode for type() | | code/h_classes.py:3:1:3:16 | ControlFlowNode for ClassExpr | code/h_classes.py:15:5:15:13 | ControlFlowNode for type() | -| code/l_calls.py:3:13:3:14 | ControlFlowNode for List | code/l_calls.py:4:12:4:12 | ControlFlowNode for x | -| code/l_calls.py:6:13:6:14 | ControlFlowNode for List | code/l_calls.py:7:16:7:16 | ControlFlowNode for x | | code/l_calls.py:12:1:12:20 | ControlFlowNode for ClassExpr | code/l_calls.py:16:16:16:18 | ControlFlowNode for cls | | code/l_calls.py:12:1:12:20 | ControlFlowNode for ClassExpr | code/l_calls.py:24:13:24:22 | ControlFlowNode for Attribute() | | code/l_calls.py:12:1:12:20 | ControlFlowNode for ClassExpr | code/l_calls.py:25:16:25:16 | ControlFlowNode for a | From a1225674ee4120bf1ab0fdeb3323b45a5da9df10 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Tue, 11 Jul 2023 11:32:26 +0200 Subject: [PATCH 09/11] Python: Add implementation note about why not targeting ESSA node --- .../lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll | 1 + 1 file changed, 1 insertion(+) diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll index 060af09edc7..a09da92ba3f 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll @@ -468,6 +468,7 @@ predicate runtimeJumpStep(Node nodeFrom, Node nodeTo) { // function, while the default value itself will be in the scope that _defines_ the // function. exists(ParameterDefinition param | + // note: we go to the _control-flow node_ of the parameter, and not the ESSA node of the parameter, since for type-tracking, the ESSA node is not a LocalSourceNode, so we would get in trouble. nodeFrom.asCfgNode() = param.getDefault() and nodeTo.asCfgNode() = param.getDefiningNode() ) From 83ca47f32c71c6d9f65d589d3e14fc3e29ecc1a3 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Tue, 11 Jul 2023 11:33:06 +0200 Subject: [PATCH 10/11] Python: Add change-note --- .../2023-07-07-parameter-default-value-definition-node.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 python/ql/lib/change-notes/2023-07-07-parameter-default-value-definition-node.md diff --git a/python/ql/lib/change-notes/2023-07-07-parameter-default-value-definition-node.md b/python/ql/lib/change-notes/2023-07-07-parameter-default-value-definition-node.md new file mode 100644 index 00000000000..3691c602500 --- /dev/null +++ b/python/ql/lib/change-notes/2023-07-07-parameter-default-value-definition-node.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Parameters with a default value are now considered a `DefinitionNode`. This improvement was motivated by allowing type-tracking and API graphs to follow flow from such a default value to a use by a captured variable. From 98ed5cf52277197024feec05908a8af01aa20ec8 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Wed, 12 Jul 2023 11:31:27 +0200 Subject: [PATCH 11/11] Python: Move `not this instanceof ParameterDefinition` logic --- python/ql/lib/semmle/python/essa/Essa.qll | 3 +-- python/ql/lib/semmle/python/essa/SsaDefinitions.qll | 7 ++++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/python/ql/lib/semmle/python/essa/Essa.qll b/python/ql/lib/semmle/python/essa/Essa.qll index 6e9d328f7d5..cf2aca1e2ac 100644 --- a/python/ql/lib/semmle/python/essa/Essa.qll +++ b/python/ql/lib/semmle/python/essa/Essa.qll @@ -501,8 +501,7 @@ class AssignmentDefinition extends EssaNodeDefinition { ControlFlowNode value; AssignmentDefinition() { - SsaSource::assignment_definition(this.getSourceVariable(), this.getDefiningNode(), value) and - not this instanceof ParameterDefinition + SsaSource::assignment_definition(this.getSourceVariable(), this.getDefiningNode(), value) } ControlFlowNode getValue() { result = value } diff --git a/python/ql/lib/semmle/python/essa/SsaDefinitions.qll b/python/ql/lib/semmle/python/essa/SsaDefinitions.qll index 61007c7129b..8d9de611659 100644 --- a/python/ql/lib/semmle/python/essa/SsaDefinitions.qll +++ b/python/ql/lib/semmle/python/essa/SsaDefinitions.qll @@ -20,7 +20,12 @@ module SsaSource { /** Holds if `v` is defined by assignment at `defn` and given `value`. */ cached predicate assignment_definition(Variable v, ControlFlowNode defn, ControlFlowNode value) { - defn.(NameNode).defines(v) and defn.(DefinitionNode).getValue() = value + defn.(NameNode).defines(v) and + defn.(DefinitionNode).getValue() = value and + // since parameter will be considered a DefinitionNode, if it has a default value, + // we need to exclude it here since it is already covered by parameter_definition + // (and points-to was unhappy that it was included in both) + not parameter_definition(v, defn) } /** Holds if `v` is defined by assignment of the captured exception. */