Skip to content

Commit f3a5d0a

Browse files
committed
Python: Refine the location of flask.request flow sources
The `flask.request` global object is commonly used in request handlers to access data in the active request. In our modelling, we handled this by treating the initial (module-local) definition of `request` as a source of remote flow. In practice this meant a lot of alerts would act as if `from flask import request` was the ultimate "source" of remote flow, and to find the actual request-handler-local instance of `request` one would have to inspect the data-flow path between source and sink. To improve this state of affairs, I have made the following changes to the definition of `FlaskRequestSource`: - We no longer consider `from flask import request` to be a source. - Instead, we look at all places where that `request` value can flow, and include only the ones that are `LocalSourceNode`s (so that inside a request handler, the first occurrence of the `request` object is the source). In practice, this leads to alerts that are much easier to decipher.
1 parent 88059d9 commit f3a5d0a

File tree

2 files changed

+15
-1
lines changed

2 files changed

+15
-1
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
5+
- The modelling of Flask requests (as sources of user-controlled data) has been improved. Rather than treating `from flask import request` as a source of remote flow, the modelling now behaves as if the first occurrence (inside a request handler) of a reference to that `request` object is a source of remote flow. This makes it much easier to understand alert messages that refer to the source of remote flow.

python/ql/lib/semmle/python/frameworks/Flask.qll

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,16 @@ module Flask {
436436
* See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Request
437437
*/
438438
private class FlaskRequestSource extends RemoteFlowSource::Range {
439-
FlaskRequestSource() { this = request().asSource() }
439+
FlaskRequestSource() {
440+
// Using `request().asSource()` would result in data-flow paths starting at the import of
441+
// `request`, which is not very useful. Instead, we look at all the places this `request`
442+
// object can flow, and use only those that are local sources (so we only get the first
443+
// instance of `request` in a function), and those that correspond to `Name` nodes (so we
444+
// don't get the initial import, which is an `ImportMember`).
445+
this = request().getAValueReachableFromSource() and
446+
this instanceof DataFlow::LocalSourceNode and
447+
this.asExpr() instanceof Name
448+
}
440449

441450
override string getSourceType() { result = "flask.request" }
442451
}

0 commit comments

Comments
 (0)