Skip to content

Commit 4226fd2

Browse files
authored
Merge pull request #20162 from Napalys/python/global_variable_tracking
Python: Add jump steps for global variable nested field access
2 parents ab5f671 + 8fd6225 commit 4226fd2

File tree

14 files changed

+226
-59
lines changed

14 files changed

+226
-59
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Data flow tracking through global variables now supports nested field access patterns such as `global_var.obj.field`. This improves the precision of taint tracking analysis when data flows through complex global variable structures.

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,15 @@ abstract class AttrRef extends Node {
6464
abstract class AttrWrite extends AttrRef {
6565
/** Gets the data flow node corresponding to the value that is written to the attribute. */
6666
abstract Node getValue();
67+
68+
/**
69+
* Holds if this attribute write writes the attribute named `attrName` on object `object` with
70+
* value `value`.
71+
*/
72+
predicate writes(Node object, string attrName, Node value) {
73+
this.accesses(object, attrName) and
74+
this.getValue() = value
75+
}
6776
}
6877

6978
/**
@@ -225,7 +234,10 @@ private class ClassDefinitionAsAttrWrite extends AttrWrite, CfgNode {
225234
* - Dynamic attribute reads using `getattr`: `getattr(object, attr)`
226235
* - Qualified imports: `from module import attr as name`
227236
*/
228-
abstract class AttrRead extends AttrRef, Node, LocalSourceNode { }
237+
abstract class AttrRead extends AttrRef, Node, LocalSourceNode {
238+
/** Holds if this attribute read reads the attribute named `attrName` on the object `object`. */
239+
predicate reads(Node object, string attrName) { this.accesses(object, attrName) }
240+
}
229241

230242
/** A simple attribute read, e.g. `object.attr` */
231243
private class AttributeReadAsAttrRead extends AttrRead, CfgNode {

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,75 @@ 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+
/** Helper predicate for `globalVariableNestedFieldJumpStep`. */
565+
pragma[nomagic]
566+
private predicate globalVariableAttrPathRead(
567+
ModuleVariableNode globalVar, string accessPath, AttrRead r, string attrName
568+
) {
569+
globalVariableAttrPathAtDepth(globalVar, accessPath, r.getObject(), _) and
570+
attrName = r.getAttributeName()
571+
}
572+
573+
/** Helper predicate for `globalVariableNestedFieldJumpStep`. */
574+
pragma[nomagic]
575+
private predicate globalVariableAttrPathWrite(
576+
ModuleVariableNode globalVar, string accessPath, AttrWrite w, string attrName
577+
) {
578+
globalVariableAttrPathAtDepth(globalVar, accessPath, w.getObject(), _) and
579+
attrName = w.getAttributeName()
580+
}
581+
582+
/**
583+
* Holds if there is a jump step from `nodeFrom` to `nodeTo` through global variable field access.
584+
* This supports tracking nested object field access through global variables like `app.obj.foo`.
585+
*/
586+
pragma[nomagic]
587+
private predicate globalVariableNestedFieldJumpStep(Node nodeFrom, Node nodeTo) {
588+
exists(ModuleVariableNode globalVar, AttrWrite write, AttrRead read |
589+
// Match writes and reads on the same global variable attribute path
590+
exists(string accessPath, string attrName |
591+
globalVariableAttrPathRead(globalVar, accessPath, read, attrName) and
592+
globalVariableAttrPathWrite(globalVar, accessPath, write, attrName)
593+
) and
594+
nodeFrom = write.getValue() and
595+
nodeTo = read
596+
)
597+
}
598+
599+
/**
600+
* Maximum depth for global variable nested attribute access.
601+
* Depth 1 = globalVar.foo, depth 2 = globalVar.foo.bar, depth 3 = globalVar.foo.bar.baz, etc.
602+
*/
603+
private int getMaxGlobalVariableDepth() { result = 2 }
604+
605+
/**
606+
* Holds if `node` is an attribute access path starting from global variable `globalVar` at specific `depth`.
607+
*/
608+
private predicate globalVariableAttrPathAtDepth(
609+
ModuleVariableNode globalVar, string accessPath, Node node, int depth
610+
) {
611+
// Base case: Direct global variable access (depth 0)
612+
depth = 0 and
613+
// We use `globalVar` instead of `globalVar.getAWrite()` due to some weirdness with how
614+
// attribute writes are handled in the global scope (see `GlobalAttributeAssignmentAsAttrWrite`).
615+
node in [globalVar.getARead(), globalVar] and
616+
accessPath = ""
617+
or
618+
exists(Node obj, string attrName, string parentAccessPath, int parentDepth |
619+
node.(AttrRead).reads(obj, attrName)
620+
or
621+
any(AttrWrite aw).writes(obj, attrName, node)
622+
|
623+
globalVariableAttrPathAtDepth(globalVar, parentAccessPath, obj, parentDepth) and
624+
accessPath = parentAccessPath + "." + attrName and
625+
depth = parentDepth + 1 and
626+
depth <= getMaxGlobalVariableDepth()
627+
)
559628
}
560629

561630
//--------

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,12 +301,12 @@ def set_to_source():
301301
@expects(4) # $ unresolved_call=expects(..) unresolved_call=expects(..)(..)
302302
def test_global_flow_to_class_attribute():
303303
inst = WithTuple2()
304-
SINK_F(WithTuple2.my_tuple[0])
304+
SINK_F(WithTuple2.my_tuple[0]) # $ SPURIOUS: flow="SOURCE, l:-5 -> WithTuple2.my_tuple[0]"
305305
SINK_F(inst.my_tuple[0])
306306

307307
set_to_source()
308308

309-
SINK(WithTuple2.my_tuple[0]) # $ MISSING: flow="SOURCE, l:-10 -> WithTuple2.my_tuple[0]"
309+
SINK(WithTuple2.my_tuple[0]) # $ flow="SOURCE, l:-10 -> WithTuple2.my_tuple[0]"
310310
SINK(inst.my_tuple[0]) # $ MISSING: flow="SOURCE, l:-11 -> inst.my_tuple[0]"
311311

312312

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ def fields_with_local_flow(x):
146146
class NestedObj(object):
147147
def __init__(self):
148148
self.obj = MyObj("OK")
149+
self.obj.foo = "default"
149150

150151
def getObj(self):
151152
return self.obj
@@ -163,17 +164,31 @@ def getObj(self):
163164
a2 = NestedObj()
164165
a2.getObj().foo = x2
165166
SINK(a2.obj.foo) # $ flow="SOURCE, l:-3 -> a2.obj.foo"
167+
168+
# Global variable
169+
app = NestedObj()
170+
171+
def init_global():
172+
app.obj.foo = SOURCE
173+
174+
def read_global():
175+
return app.obj.foo
176+
177+
def test_global_nested_attributes():
178+
init_global()
179+
result = read_global()
180+
SINK(result) # $ flow="SOURCE, l:-8 -> result"
166181

167182
# ------------------------------------------------------------------------------
168183
# Global scope interaction
169184
# ------------------------------------------------------------------------------
170185

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

174189
global_obj = MyObj(NONSOURCE)
175190
global_obj.foo = SOURCE
176191
SINK(global_obj.foo) # $ flow="SOURCE, l:-1 -> global_obj.foo"
177192

178193
def func_defined_after():
179-
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/DataflowQueryTest.expected

Lines changed: 0 additions & 2 deletions
This file was deleted.

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

Lines changed: 0 additions & 4 deletions
This file was deleted.

0 commit comments

Comments
 (0)