Skip to content

Commit b12c67f

Browse files
authored
Merge pull request #22092 from github/tausbn/python-hotfix-disable-instance-field-step
Python: hotfix - disable instanceFieldStep to avoid type-tracker blowup
2 parents 7c73de0 + f251a57 commit b12c67f

5 files changed

Lines changed: 18 additions & 13 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
5+
- Temporarily disabled the `instanceFieldStep` disjunct of the internal `TypeTrackingInput::levelStepCall` predicate, which was introduced in 7.2.0 and caused catastrophic query slowdowns on some OOP-heavy Python codebases (e.g. `mypy` and `dask`).

python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackingImpl.qll

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,13 @@ module TypeTrackingInput implements Shared::TypeTrackingInput<Location> {
170170

171171
/** Holds if there is a level step from `nodeFrom` to `nodeTo`, which may depend on the call graph. */
172172
predicate levelStepCall(Node nodeFrom, LocalSourceNode nodeTo) {
173-
instanceFieldStep(nodeFrom, nodeTo)
173+
// HOTFIX: `instanceFieldStep` is temporarily disabled (via `and none()`).
174+
// It uses `classInstanceTracker(cls)` -- itself a type-tracker run --
175+
// from inside `levelStepCall`, creating a structural mutual recursion
176+
// that causes catastrophic query slowdowns on some OOP-heavy Python
177+
// codebases (e.g. mypy and dask). The `and none()` should be removed
178+
// once that recursion is redesigned.
179+
instanceFieldStep(nodeFrom, nodeTo) and none()
174180
or
175181
inheritedFieldStep(nodeFrom, nodeTo)
176182
}

python/ql/test/library-tests/dataflow/typetracking/attribute_tests.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ def possibly_uncalled_method(self): # $ MISSING: tracked=foo
157157
print(self.foo) # $ tracked MISSING: tracked=foo
158158

159159
instance = MyClass2()
160-
print(instance.foo) # $ tracked MISSING: tracked=foo
160+
print(instance.foo) # $ MISSING: tracked=foo tracked
161161
instance.print_foo() # $ MISSING: tracked=foo
162162

163163

@@ -195,7 +195,7 @@ def read_foo(self): # $ MISSING: tracked=foo
195195

196196
sub1 = Sub1()
197197
sub1.read_foo()
198-
print(sub1.foo) # $ tracked MISSING: tracked=foo
198+
print(sub1.foo) # $ MISSING: tracked=foo tracked
199199

200200

201201
# attribute written in a subclass method, read in an inherited base class method
@@ -210,7 +210,7 @@ def __init__(self): # $ tracked=bar
210210

211211
sub2 = Sub2()
212212
sub2.read_bar()
213-
print(sub2.bar) # $ tracked MISSING: tracked=bar
213+
print(sub2.bar) # $ MISSING: tracked=bar tracked
214214

215215

216216
# attribute written in a base class method, read on an instance of the subclass
@@ -223,4 +223,4 @@ class Sub3(Base3):
223223
pass
224224

225225
sub3 = Sub3()
226-
print(sub3.baz) # $ tracked MISSING: tracked=baz
226+
print(sub3.baz) # $ MISSING: tracked=baz tracked

python/ql/test/query-tests/Security/CWE-089-SqlInjection/SqlInjection.expected

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#select
22
| app.py:23:20:23:24 | ControlFlowNode for query | app.py:20:18:20:21 | ControlFlowNode for name | app.py:23:20:23:24 | ControlFlowNode for query | This SQL query depends on a $@. | app.py:20:18:20:21 | ControlFlowNode for name | user-provided value |
33
| app.py:30:20:30:24 | ControlFlowNode for query | app.py:27:19:27:22 | ControlFlowNode for name | app.py:30:20:30:24 | ControlFlowNode for query | This SQL query depends on a $@. | app.py:27:19:27:22 | ControlFlowNode for name | user-provided value |
4-
| app.py:37:20:37:24 | ControlFlowNode for query | app.py:34:19:34:22 | ControlFlowNode for name | app.py:37:20:37:24 | ControlFlowNode for query | This SQL query depends on a $@. | app.py:34:19:34:22 | ControlFlowNode for name | user-provided value |
54
| app.py:44:20:44:24 | ControlFlowNode for query | app.py:41:19:41:22 | ControlFlowNode for name | app.py:44:20:44:24 | ControlFlowNode for query | This SQL query depends on a $@. | app.py:41:19:41:22 | ControlFlowNode for name | user-provided value |
65
| app.py:51:20:51:24 | ControlFlowNode for query | app.py:48:19:48:22 | ControlFlowNode for name | app.py:51:20:51:24 | ControlFlowNode for query | This SQL query depends on a $@. | app.py:48:19:48:22 | ControlFlowNode for name | user-provided value |
76
| sql_injection.py:21:24:21:77 | ControlFlowNode for BinaryExpr | sql_injection.py:14:15:14:22 | ControlFlowNode for username | sql_injection.py:21:24:21:77 | ControlFlowNode for BinaryExpr | This SQL query depends on a $@. | sql_injection.py:14:15:14:22 | ControlFlowNode for username | user-provided value |
@@ -25,8 +24,6 @@ edges
2524
| app.py:21:5:21:9 | ControlFlowNode for query | app.py:23:20:23:24 | ControlFlowNode for query | provenance | |
2625
| app.py:27:19:27:22 | ControlFlowNode for name | app.py:28:5:28:9 | ControlFlowNode for query | provenance | |
2726
| app.py:28:5:28:9 | ControlFlowNode for query | app.py:30:20:30:24 | ControlFlowNode for query | provenance | |
28-
| app.py:34:19:34:22 | ControlFlowNode for name | app.py:35:5:35:9 | ControlFlowNode for query | provenance | |
29-
| app.py:35:5:35:9 | ControlFlowNode for query | app.py:37:20:37:24 | ControlFlowNode for query | provenance | |
3027
| app.py:41:19:41:22 | ControlFlowNode for name | app.py:42:5:42:9 | ControlFlowNode for query | provenance | |
3128
| app.py:42:5:42:9 | ControlFlowNode for query | app.py:44:20:44:24 | ControlFlowNode for query | provenance | |
3229
| app.py:48:19:48:22 | ControlFlowNode for name | app.py:49:5:49:9 | ControlFlowNode for query | provenance | |
@@ -54,9 +51,6 @@ nodes
5451
| app.py:27:19:27:22 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
5552
| app.py:28:5:28:9 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
5653
| app.py:30:20:30:24 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
57-
| app.py:34:19:34:22 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
58-
| app.py:35:5:35:9 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
59-
| app.py:37:20:37:24 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
6054
| app.py:41:19:41:22 | ControlFlowNode for name | semmle.label | ControlFlowNode for name |
6155
| app.py:42:5:42:9 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |
6256
| app.py:44:20:44:24 | ControlFlowNode for query | semmle.label | ControlFlowNode for query |

python/ql/test/query-tests/Security/CWE-089-SqlInjection/app.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ async def unsafe2(name: str): # $ Source
3131
cursor.close()
3232

3333
@app.get("/unsafe3/")
34-
async def unsafe3(name: str): # $ Source
34+
async def unsafe3(name: str): # $ MISSING: Source
3535
query = "select * from users where name=" + name
3636
cursor = hdb_con3.cursor()
37-
cursor.execute(query) # $ Alert
37+
cursor.execute(query) # $ MISSING: Alert
3838
cursor.close()
3939

4040
@app.get("/unsafe4/")

0 commit comments

Comments
 (0)