Skip to content

Commit 6e9d9fc

Browse files
committed
Python: Improve taint steps in for & iterable unpacking
These were written way before the ones in DataFlowPrivate, but apparently didn't cover quite as much :|
1 parent d3163d8 commit 6e9d9fc

File tree

5 files changed

+8
-31
lines changed

5 files changed

+8
-31
lines changed

python/ql/src/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,11 @@ private module Cached {
4646
or
4747
copyStep(nodeFrom, nodeTo)
4848
or
49-
forStep(nodeFrom, nodeTo)
49+
DataFlowPrivate::forReadStep(nodeFrom, _, nodeTo)
5050
or
51-
unpackingAssignmentStep(nodeFrom, nodeTo)
51+
DataFlowPrivate::iterableUnpackingReadStep(nodeFrom, _, nodeTo)
52+
or
53+
DataFlowPrivate::iterableUnpackingStoreStep(nodeFrom, _, nodeTo)
5254
}
5355
}
5456

@@ -199,28 +201,3 @@ predicate copyStep(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeTo) {
199201
call.getArg(0) = nodeFrom
200202
)
201203
}
202-
203-
/**
204-
* Holds if taint can flow from `nodeFrom` to `nodeTo` with a step related to `for`-iteration,
205-
* for example `for x in xs`, or `for x,y in points`.
206-
*/
207-
predicate forStep(DataFlow::CfgNode nodeFrom, DataFlow::EssaNode nodeTo) {
208-
exists(EssaNodeDefinition defn, For for |
209-
for.getTarget().getAChildNode*() = defn.getDefiningNode().getNode() and
210-
nodeTo.getVar() = defn and
211-
nodeFrom.asExpr() = for.getIter()
212-
)
213-
}
214-
215-
/**
216-
* Holds if taint can flow from `nodeFrom` to `nodeTo` with a step related to iterable unpacking.
217-
* Only handles normal assignment (`x,y = calc_point()`), since `for x,y in points` is handled by `forStep`.
218-
*/
219-
predicate unpackingAssignmentStep(DataFlow::CfgNode nodeFrom, DataFlow::EssaNode nodeTo) {
220-
// `a, b = myiterable` or `head, *tail = myiterable` (only Python 3)
221-
exists(MultiAssignmentDefinition defn, Assign assign |
222-
assign.getATarget().contains(defn.getDefiningNode().getNode()) and
223-
nodeTo.getVar() = defn and
224-
nodeFrom.asExpr() = assign.getValue()
225-
)
226-
}

python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/test_collections.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def test_access(x, y, z):
5252
reversed(tainted_list), # $ tainted
5353
iter(tainted_list), # $ tainted
5454
next(iter(tainted_list)), # $ tainted
55-
[i for i in tainted_list], # $ MISSING: tainted
55+
[i for i in tainted_list], # $ tainted
5656
[tainted_list for _i in [1,2,3]], # $ MISSING: tainted
5757
)
5858

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ async def test_taint(request: web.Request): # $ requestHandler
5555
await request.content.readline(), # $ tainted
5656
await request.content.readchunk(), # $ tainted
5757
(await request.content.readchunk())[0], # $ tainted
58-
[line async for line in request.content], # $ MISSING: tainted
58+
[line async for line in request.content], # $ tainted
5959
[data async for data in request.content.iter_chunked(1024)], # $ MISSING: tainted
6060
[data async for data in request.content.iter_any()], # $ MISSING: tainted
6161
[data async for data, _ in request.content.iter_chunks()], # $ MISSING: tainted

python/ql/test/library-tests/frameworks/django-v2-v3/taint_test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ def test_taint(request: HttpRequest, foo, bar, baz=None): # $requestHandler rou
108108
request.readline(), # $ tainted
109109
request.readlines(), # $ tainted
110110
request.readlines()[0], # $ tainted
111-
[line for line in request], # $ MISSING: tainted
111+
[line for line in request], # $ tainted
112112
)
113113

114114
# django.urls.ResolverMatch also supports iterable unpacking

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ def get(self, name = "World!", number="0", foo="foo"): # $ requestHandler route
6363
request.headers["header-name"], # $ tainted
6464
request.headers.get_list("header-name"), # $ tainted
6565
request.headers.get_all(), # $ tainted
66-
[(k, v) for (k, v) in request.headers.get_all()], # $ MISSING: tainted
66+
[(k, v) for (k, v) in request.headers.get_all()], # $ tainted
6767

6868
# Dict[str, http.cookies.Morsel]
6969
request.cookies, # $ tainted

0 commit comments

Comments
 (0)