Skip to content

Commit e9acea8

Browse files
committed
Python: Improve multidict modeling
1 parent 2e851cd commit e9acea8

File tree

8 files changed

+71
-25
lines changed

8 files changed

+71
-25
lines changed

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ module Multidict {
2424
* See https://multidict.readthedocs.io/en/stable/multidict.html#multidictproxy
2525
*/
2626
module MultiDictProxy {
27+
/** Gets a reference to a `MultiDictProxy` class. */
28+
API::Node classRef() {
29+
result = API::moduleImport("multidict").getMember(["MultiDictProxy", "CIMultiDictProxy"])
30+
}
31+
2732
/**
2833
* A source of instances of `multidict.MultiDictProxy`, extend this class to model
2934
* new instances.
@@ -37,15 +42,20 @@ module Multidict {
3742
*/
3843
abstract class InstanceSource extends DataFlow::LocalSourceNode { }
3944

40-
/** Gets a reference to an instance of `multidict.MultiDictProxy`. */
45+
/** A direct instantiation of a `MultiDictProxy` class. */
46+
private class ClassInstantiation extends InstanceSource, DataFlow::CallCfgNode {
47+
ClassInstantiation() { this = classRef().getACall() }
48+
}
49+
50+
/** Gets a reference to an instance of a `MultiDictProxy` class. */
4151
private DataFlow::LocalSourceNode instance(DataFlow::TypeTracker t) {
4252
t.start() and
4353
result instanceof InstanceSource
4454
or
4555
exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t))
4656
}
4757

48-
/** Gets a reference to an instance of `multidict.MultiDictProxy`. */
58+
/** Gets a reference to an instance of a `MultiDictProxy` class. */
4959
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
5060

5161
/**
@@ -55,6 +65,12 @@ module Multidict {
5565
*/
5666
class MultiDictProxyAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
5767
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
68+
// class instantiation
69+
exists(ClassInstantiation call |
70+
nodeFrom = call.getArg(0) and
71+
nodeTo = call
72+
)
73+
or
5874
// Methods
5975
//
6076
// TODO: When we have tools that make it easy, model these properly to handle

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

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ async def test_taint(request: web.Request): # $ requestHandler
55
ensure_tainted(
66
request, # $ tainted
77

8-
# yarl.URL instances, see tests under `yarl` framework tests
9-
# https://yarl.readthedocs.io/en/stable/api.html#yarl.URL
8+
# yarl.URL (see `yarl` framework tests)
109
request.url, # $ tainted
1110
request.url.human_repr(), # $ tainted
1211
request.rel_url, # $ tainted
@@ -25,28 +24,11 @@ async def test_taint(request: web.Request): # $ requestHandler
2524
request.match_info["key"], # $ tainted
2625
request.match_info.get("key"), # $ tainted
2726

28-
# multidict.MultiDictProxy[str]
29-
# see https://multidict.readthedocs.io/en/stable/multidict.html#multidict.MultiDictProxy
30-
# TODO: Should have a better way to capture that we in fact _do_ model this as a
31-
# an instance of the right class, and have the actual taint_test for that in a
32-
# different file!
27+
# multidict.MultiDictProxy[str] (see `multidict` framework tests)
3328
request.query, # $ tainted
34-
request.query["key"], # $ tainted
35-
request.query.get("key"), # $ tainted
3629
request.query.getone("key"), # $ tainted
37-
request.query.getall("key"), # $ tainted
38-
request.query.keys(), # $ MISSING: tainted
39-
request.query.values(), # $ tainted
40-
request.query.items(), # $ tainted
41-
request.query.copy(), # $ tainted
42-
list(request.query), # $ tainted
43-
iter(request.query), # $ tainted
44-
45-
# multidict.CIMultiDictProxy[str]
46-
# see https://multidict.readthedocs.io/en/stable/multidict.html#multidict.CIMultiDictProxy
47-
# TODO: Should have a better way to capture that we in fact _do_ model this as a
48-
# an instance of the right class, and have the actual taint_test for that in a
49-
# different file!
30+
31+
# multidict.CIMultiDictProxy[str] (see `multidict` framework tests)
5032
request.headers, # $ tainted
5133
request.headers.getone("key"), # $ tainted
5234

@@ -99,7 +81,7 @@ async def test_taint(request: web.Request): # $ requestHandler
9981
# aiohttp.multipart.MultipartReader
10082
await request.multipart(), # $ tainted
10183

102-
# multidict.MultiDictProxy[str]
84+
# multidict.MultiDictProxy[str] (see `multidict` framework tests)
10385
await request.post(), # $ tainted
10486
(await request.post()).getone("key"), # $ MISSING: tainted
10587
)

python/ql/test/library-tests/frameworks/multidict/ConceptsTest.expected

Whitespace-only changes.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import python
2+
import experimental.meta.ConceptsTest
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
argumentToEnsureNotTaintedNotMarkedAsSpurious
2+
untaintedArgumentToEnsureTaintedNotMarkedAsMissing
3+
failures
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import experimental.meta.InlineTaintTest
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
semmle-extractor-options: --max-import-depth=1 --lang=3
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import multidict
2+
3+
# TODO: This is an invalid MultiDictProxy construction... but for the purpose of
4+
# taint-test, this should be good enough.
5+
mdp = multidict.MultiDictProxy(TAINTED_STRING)
6+
7+
ensure_tainted(
8+
# see https://multidict.readthedocs.io/en/stable/multidict.html#multidict.MultiDictProxy
9+
10+
mdp, # $ tainted
11+
mdp["key"], # $ tainted
12+
mdp.get("key"), # $ tainted
13+
mdp.getone("key"), # $ tainted
14+
mdp.getall("key"), # $ tainted
15+
mdp.keys(), # $ MISSING: tainted
16+
mdp.values(), # $ tainted
17+
mdp.items(), # $ tainted
18+
mdp.copy(), # $ tainted
19+
list(mdp), # $ tainted
20+
iter(mdp), # $ tainted
21+
)
22+
23+
# TODO: This is an invalid CIMultiDictProxy construction... but for the purpose of
24+
# taint-test, this should be good enough.
25+
ci_mdp = multidict.CIMultiDictProxy(TAINTED_STRING)
26+
27+
ensure_tainted(
28+
# see https://multidict.readthedocs.io/en/stable/multidict.html#multidict.CIMultiDictProxy
29+
30+
ci_mdp, # $ tainted
31+
ci_mdp["key"], # $ tainted
32+
ci_mdp.get("key"), # $ tainted
33+
ci_mdp.getone("key"), # $ tainted
34+
ci_mdp.getall("key"), # $ tainted
35+
ci_mdp.keys(), # $ MISSING: tainted
36+
ci_mdp.values(), # $ tainted
37+
ci_mdp.items(), # $ tainted
38+
ci_mdp.copy(), # $ tainted
39+
list(ci_mdp), # $ tainted
40+
iter(ci_mdp), # $ tainted
41+
)

0 commit comments

Comments
 (0)