Skip to content

Commit e60d0c8

Browse files
NapalysNapalys
committed
Python: Add global variable nested field jump steps
1 parent 9d4b168 commit e60d0c8

File tree

4 files changed

+64
-4
lines changed

4 files changed

+64
-4
lines changed

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,61 @@ predicate runtimeJumpStep(Node nodeFrom, Node nodeTo) {
556556
nodeFrom.asCfgNode() = param.getDefault() and
557557
nodeTo.asCfgNode() = param.getDefiningNode()
558558
)
559+
or
560+
// Enhanced global variable field access tracking
561+
globalVariableNestedFieldJumpStep(nodeFrom, nodeTo)
562+
}
563+
564+
/**
565+
* Holds if there is a jump step from `nodeFrom` to `nodeTo` through global variable field access.
566+
* This supports tracking nested object field access through global variables like `app.obj.foo`.
567+
*/
568+
predicate globalVariableNestedFieldJumpStep(Node nodeFrom, Node nodeTo) {
569+
exists(GlobalVariable globalVar, AttrWrite write, AttrRead read |
570+
// Match writes and reads on the same global variable attribute path
571+
globalVariableAttrPath(globalVar, write.getObject()) and
572+
globalVariableAttrPath(globalVar, read.getObject()) and
573+
write.getAttributeName() = read.getAttributeName() and
574+
nodeFrom = write.getValue() and
575+
nodeTo = read and
576+
write.getEnclosingCallable() != read.getEnclosingCallable()
577+
)
578+
}
579+
580+
/**
581+
* Maximum depth for global variable nested attribute access.
582+
* Depth 0 = globalVar.foo, depth 1 = globalVar.foo.bar, depth 2 = globalVar.foo.bar.baz, etc.
583+
*/
584+
private int getMaxGlobalVariableDepth() { result = 1 }
585+
586+
/**
587+
* Holds if `node` is an attribute access path starting from global variable `globalVar`.
588+
* Supports configurable nesting depth via getMaxGlobalVariableDepth().
589+
*/
590+
predicate globalVariableAttrPath(GlobalVariable globalVar, Node node) {
591+
globalVariableAttrPathAtDepth(globalVar, node, _)
592+
}
593+
594+
/**
595+
* Holds if `node` is an attribute access path starting from global variable `globalVar` at specific `depth`.
596+
*/
597+
predicate globalVariableAttrPathAtDepth(GlobalVariable globalVar, Node node, int depth) {
598+
// Base case: Direct global variable access (depth 0)
599+
depth = 0 and
600+
exists(NameNode name |
601+
name.getId() = globalVar.getId() and
602+
node.asCfgNode() = name and
603+
name.getNode().(Name).getVariable() instanceof GlobalVariable and
604+
not exists(ClassExpr cls | cls.getName() = globalVar.getId())
605+
)
606+
or
607+
// Recursive case: Nested attribute access (depth > 0)
608+
exists(AttrRead attr, int parentDepth |
609+
globalVariableAttrPathAtDepth(globalVar, attr.getObject(), parentDepth) and
610+
node = attr and
611+
depth = parentDepth + 1 and
612+
depth <= getMaxGlobalVariableDepth()
613+
)
559614
}
560615

561616
//--------

‎python/ql/test/library-tests/dataflow/fieldflow/test_global.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,18 +177,18 @@ def read_global():
177177
def test_global_nested_attributes():
178178
init_global()
179179
result = read_global()
180-
SINK(result) # $ MISSING: flow="SOURCE, l:-8 -> result"
180+
SINK(result) # $ flow="SOURCE, l:-8 -> result"
181181

182182
# ------------------------------------------------------------------------------
183183
# Global scope interaction
184184
# ------------------------------------------------------------------------------
185185

186186
def func_defined_before():
187-
SINK(global_obj.foo) # $ MISSING: flow="SOURCE, l:+3 -> global_obj.foo"
187+
SINK(global_obj.foo) # $ flow="SOURCE, l:+3 -> global_obj.foo"
188188

189189
global_obj = MyObj(NONSOURCE)
190190
global_obj.foo = SOURCE
191191
SINK(global_obj.foo) # $ flow="SOURCE, l:-1 -> global_obj.foo"
192192

193193
def func_defined_after():
194-
SINK(global_obj.foo) # $ MISSING: flow="SOURCE, l:-4 -> global_obj.foo"
194+
SINK(global_obj.foo) # $ flow="SOURCE, l:-4 -> global_obj.foo"

‎python/ql/test/query-tests/Security/CWE-022-PathInjection/PathInjection.expected‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
| fastapi_path_injection.py:7:19:7:26 | ControlFlowNode for filepath | fastapi_path_injection.py:17:21:17:24 | ControlFlowNode for path | fastapi_path_injection.py:7:19:7:26 | ControlFlowNode for filepath | This path depends on a $@. | fastapi_path_injection.py:17:21:17:24 | ControlFlowNode for path | user-provided value |
33
| fastapi_path_injection.py:7:19:7:26 | ControlFlowNode for filepath | fastapi_path_injection.py:26:21:26:24 | ControlFlowNode for path | fastapi_path_injection.py:7:19:7:26 | ControlFlowNode for filepath | This path depends on a $@. | fastapi_path_injection.py:26:21:26:24 | ControlFlowNode for path | user-provided value |
44
| fastapi_path_injection.py:7:19:7:26 | ControlFlowNode for filepath | fastapi_path_injection.py:31:21:31:24 | ControlFlowNode for path | fastapi_path_injection.py:7:19:7:26 | ControlFlowNode for filepath | This path depends on a $@. | fastapi_path_injection.py:31:21:31:24 | ControlFlowNode for path | user-provided value |
5+
| fastapi_path_injection.py:7:19:7:26 | ControlFlowNode for filepath | fastapi_path_injection.py:48:21:48:24 | ControlFlowNode for path | fastapi_path_injection.py:7:19:7:26 | ControlFlowNode for filepath | This path depends on a $@. | fastapi_path_injection.py:48:21:48:24 | ControlFlowNode for path | user-provided value |
56
| flask_path_injection.py:21:32:21:38 | ControlFlowNode for dirname | flask_path_injection.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_path_injection.py:21:32:21:38 | ControlFlowNode for dirname | This path depends on a $@. | flask_path_injection.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
67
| path_injection.py:13:14:13:47 | ControlFlowNode for Attribute() | path_injection.py:3:26:3:32 | ControlFlowNode for ImportMember | path_injection.py:13:14:13:47 | ControlFlowNode for Attribute() | This path depends on a $@. | path_injection.py:3:26:3:32 | ControlFlowNode for ImportMember | user-provided value |
78
| path_injection.py:21:14:21:18 | ControlFlowNode for npath | path_injection.py:3:26:3:32 | ControlFlowNode for ImportMember | path_injection.py:21:14:21:18 | ControlFlowNode for npath | This path depends on a $@. | path_injection.py:3:26:3:32 | ControlFlowNode for ImportMember | user-provided value |
@@ -30,6 +31,8 @@ edges
3031
| fastapi_path_injection.py:27:34:27:37 | ControlFlowNode for path | fastapi_path_injection.py:6:24:6:31 | ControlFlowNode for filepath | provenance | |
3132
| fastapi_path_injection.py:31:21:31:24 | ControlFlowNode for path | fastapi_path_injection.py:32:34:32:37 | ControlFlowNode for path | provenance | |
3233
| fastapi_path_injection.py:32:34:32:37 | ControlFlowNode for path | fastapi_path_injection.py:6:24:6:31 | ControlFlowNode for filepath | provenance | |
34+
| fastapi_path_injection.py:48:21:48:24 | ControlFlowNode for path | fastapi_path_injection.py:49:45:49:48 | ControlFlowNode for path | provenance | |
35+
| fastapi_path_injection.py:49:45:49:48 | ControlFlowNode for path | fastapi_path_injection.py:6:24:6:31 | ControlFlowNode for filepath | provenance | |
3336
| flask_path_injection.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_path_injection.py:1:26:1:32 | ControlFlowNode for request | provenance | |
3437
| flask_path_injection.py:1:26:1:32 | ControlFlowNode for request | flask_path_injection.py:19:15:19:21 | ControlFlowNode for request | provenance | |
3538
| flask_path_injection.py:19:5:19:11 | ControlFlowNode for dirname | flask_path_injection.py:21:32:21:38 | ControlFlowNode for dirname | provenance | |
@@ -161,6 +164,8 @@ nodes
161164
| fastapi_path_injection.py:27:34:27:37 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
162165
| fastapi_path_injection.py:31:21:31:24 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
163166
| fastapi_path_injection.py:32:34:32:37 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
167+
| fastapi_path_injection.py:48:21:48:24 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
168+
| fastapi_path_injection.py:49:45:49:48 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
164169
| flask_path_injection.py:1:26:1:32 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember |
165170
| flask_path_injection.py:1:26:1:32 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
166171
| flask_path_injection.py:19:5:19:11 | ControlFlowNode for dirname | semmle.label | ControlFlowNode for dirname |

‎python/ql/test/query-tests/Security/CWE-022-PathInjection/fastapi_path_injection.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,5 @@ async def read_item(path: str, data_source=Depends(get_data_source)): # $ MISSIN
4545
return data_source.get_data(path)
4646

4747
@app.get("/file5/", dependencies=[Depends(init_file_handler)])
48-
async def read_item(path: str): # $ MISSING: Source
48+
async def read_item(path: str): # $ Source
4949
return app.state.file_handler2.get_data(path)

0 commit comments

Comments
 (0)