Skip to content

Commit 6f63c03

Browse files
committed
Python: Model http.cookies.Morsel and usage in Tornado
1 parent 7e09a1c commit 6f63c03

File tree

3 files changed

+75
-2
lines changed

3 files changed

+75
-2
lines changed

python/ql/src/semmle/python/frameworks/Stdlib.qll

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,59 @@ module Stdlib {
115115
}
116116
}
117117
}
118+
119+
/**
120+
* Provides models for the `http.cookies.Morsel` class
121+
*
122+
* See https://docs.python.org/3.9/library/http.cookies.html#http.cookies.Morsel.
123+
*/
124+
module Morsel {
125+
/**
126+
* A source of instances of `http.cookies.Morsel`, extend this class to model new instances.
127+
*
128+
* This can include instantiations of the class, return values from function
129+
* calls, or a special parameter that will be set when functions are called by an external
130+
* library.
131+
*
132+
* Use the predicate `Morsel::instance()` to get references to instances of `http.cookies.Morsel`.
133+
*/
134+
abstract class InstanceSource extends DataFlow::LocalSourceNode { }
135+
136+
/** Gets a reference to an instance of `http.cookies.Morsel`. */
137+
private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) {
138+
t.start() and
139+
result instanceof InstanceSource
140+
or
141+
exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t))
142+
}
143+
144+
/** Gets a reference to an instance of `http.cookies.Morsel`. */
145+
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
146+
147+
/**
148+
* Taint propagation for `http.cookies.Morsel`.
149+
*/
150+
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
151+
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
152+
// Methods
153+
//
154+
// TODO: When we have tools that make it easy, model these properly to handle
155+
// `meth = obj.meth; meth()`. Until then, we'll use this more syntactic approach
156+
// (since it allows us to at least capture the most common cases).
157+
nodeFrom = instance() and
158+
exists(DataFlow::AttrRead attr | attr.getObject() = nodeFrom |
159+
// normal (non-async) methods
160+
attr.getAttributeName() in ["output", "js_output"] and
161+
nodeTo.(DataFlow::CallCfgNode).getFunction() = attr
162+
)
163+
or
164+
// Attributes
165+
nodeFrom = instance() and
166+
nodeTo.(DataFlow::AttrRead).getObject() = nodeFrom and
167+
nodeTo.(DataFlow::AttrRead).getAttributeName() in ["key", "value", "coded_value"]
168+
}
169+
}
170+
}
118171
}
119172

120173
/**

python/ql/src/semmle/python/frameworks/Tornado.qll

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ private import semmle.python.dataflow.new.TaintTracking
1010
private import semmle.python.Concepts
1111
private import semmle.python.ApiGraphs
1212
private import semmle.python.regex
13+
private import semmle.python.frameworks.Stdlib
1314

1415
/**
1516
* Provides models for the `tornado` PyPI package.
@@ -367,6 +368,24 @@ private module Tornado {
367368
this.(DataFlow::AttrRead).accesses(instance(), "headers")
368369
}
369370
}
371+
372+
/** An `Morsel` instance that originates from a Tornado request. */
373+
private class TornadoRequestMorselInstances extends Stdlib::Morsel::InstanceSource {
374+
TornadoRequestMorselInstances() {
375+
// TODO: this currently only works in local-scope, since writing type-trackers for
376+
// this is a little too much effort. Once API-graphs are available for more
377+
// things, we can rewrite this.
378+
//
379+
// TODO: This approach for identifying member-access is very adhoc, and we should
380+
// be able to do something more structured for providing modeling of the members
381+
// of a container-object.
382+
exists(DataFlow::AttrRead files | files.accesses(instance(), "cookies") |
383+
this.asCfgNode().(SubscriptNode).getObject() = files.asCfgNode()
384+
or
385+
this.(DataFlow::MethodCallNode).calls(files, "get")
386+
)
387+
}
388+
}
370389
}
371390
}
372391
}

python/ql/test/library-tests/frameworks/tornado/taint_test.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,9 @@ def get(self, name = "World!", number="0", foo="foo"): # $ requestHandler route
6868
# Dict[str, http.cookies.Morsel]
6969
request.cookies, # $ tainted
7070
request.cookies["cookie-name"], # $ tainted
71-
request.cookies["cookie-name"].key, # $ MISSING: tainted
72-
request.cookies["cookie-name"].value, # $ MISSING: tainted
71+
request.cookies["cookie-name"].key, # $ tainted
72+
request.cookies["cookie-name"].value, # $ tainted
73+
request.cookies["cookie-name"].coded_value, # $ tainted
7374
)
7475

7576

0 commit comments

Comments
 (0)