Skip to content

Commit dd61dd2

Browse files
committed
Fix FP for py/modification-of-locals
1 parent 47c2c9e commit dd61dd2

3 files changed

Lines changed: 15 additions & 8 deletions

File tree

python/ql/src/Statements/ModificationOfLocals.ql

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,19 @@
1313

1414
import python
1515
private import semmle.python.ApiGraphs
16+
private import semmle.python.dataflow.new.DataFlow
1617

1718
predicate originIsLocals(ControlFlowNode n) {
18-
API::builtin("locals").getReturn().getAValueReachableFromSource().asCfgNode() = n
19+
// Only consider the `locals()` dictionary within the scope that called `locals()`.
20+
// Once the dictionary is passed to another scope (e.g. as an argument or via an
21+
// instance attribute) it is just an ordinary mapping, and modifying it is both
22+
// meaningful and effective. Restricting to local (intraprocedural) flow ensures we
23+
// only report modifications in the scope where the `locals()` gotcha actually applies.
24+
exists(DataFlow::LocalSourceNode src, DataFlow::Node use |
25+
src = API::builtin("locals").getReturn().asSource() and
26+
src.flowsTo(use) and
27+
use.asCfgNode() = n
28+
)
1929
}
2030

2131
predicate modification_of_locals(ControlFlowNode f) {
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `py/modification-of-locals` query no longer flags modifications of a `locals()` dictionary that has been passed out of the scope in which `locals()` was called (for example, by passing it to another function or storing it in an instance attribute). In such cases the dictionary is used as an ordinary mapping and modifying it is meaningful, so these were false positives. The "modification has no effect" claim only applies within the scope that called `locals()`, which is now the only case reported.

python/ql/test/query-tests/Statements/general/ModificationOfLocals.expected

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,3 @@
33
| test.py:101:5:101:14 | Attribute() | Modification of the locals() dictionary will have no effect on the local variables. |
44
| test.py:102:9:102:14 | Subscript | Modification of the locals() dictionary will have no effect on the local variables. |
55
| test.py:103:5:103:13 | Attribute() | Modification of the locals() dictionary will have no effect on the local variables. |
6-
| test.py:206:5:206:11 | Subscript | Modification of the locals() dictionary will have no effect on the local variables. |
7-
| test.py:207:5:207:23 | Attribute() | Modification of the locals() dictionary will have no effect on the local variables. |
8-
| test.py:208:5:208:15 | Attribute() | Modification of the locals() dictionary will have no effect on the local variables. |
9-
| test.py:209:9:209:15 | Subscript | Modification of the locals() dictionary will have no effect on the local variables. |
10-
| test.py:210:5:210:14 | Attribute() | Modification of the locals() dictionary will have no effect on the local variables. |
11-
| test.py:228:9:228:24 | Subscript | Modification of the locals() dictionary will have no effect on the local variables. |
12-
| test.py:229:9:229:35 | Attribute() | Modification of the locals() dictionary will have no effect on the local variables. |

0 commit comments

Comments
 (0)