hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 17:23:36 +01:00
Code Issues Packages Projects Releases Wiki Activity

Python: Align flask taint modeling with rest of code

Browse Source 
This was a good time to do this, so we don't have 2 different ways of doing the
same thing.

I needed to do this to figure out if we should expose
`API::moduleImport("flask").getMember("request")` in a helper predicate or
not. I think I ended up using more refenreces to this in the end. Although it's
not unreasonable to let someone do this themselves, I also think it's reasonable
that we provide a helper predicate for this.
This commit is contained in:
Rasmus Wriedt Larsen
2021-02-18 15:04:07 +01:00
parent ba61099172
commit 19b7ea8d85
1 changed files with 66 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

78
python/ql/src/semmle/python/frameworks/Flask.qll
View File

@@ -74,14 +74,8 @@ private module FlaskModel {
API::Node instance() { result = classRef().getReturn() }
}
// ---------------------------------------------------------------------------
// flask
// ---------------------------------------------------------------------------
/** Provides models for the `flask` module. */
module flask {
/** Gets a reference to the `flask.request` object. */
API::Node request() { result = API::moduleImport("flask").getMember("request") }
}
/**
* Provides models for the `flask.Response` class
@@ -305,42 +299,43 @@ private module FlaskModel {
// ---------------------------------------------------------------------------
// flask.Request taint modeling
// ---------------------------------------------------------------------------
// TODO: Do we even need this class? :|
/**
* A source of remote flow from a flask request.
*
* See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Request
*/
private class RequestSource extends RemoteFlowSource::Range {
RequestSource() { this = flask::request().getAUse() }
RequestSource() { this = request().getAUse() }
override string getSourceType() { result = "flask.request" }
}
private module FlaskRequestTracking {
/** Gets a reference to either of the `get_json` or `get_data` attributes of a Flask request. */
API::Node tainted_methods(string attr_name) {
attr_name in ["get_data", "get_json"] and
result = flask::request().getMember(attr_name)
}
}
/**
* A source of remote flow from attributes from a flask request.
* Taint propagation for a flask request.
*
* See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Request
*/
private class RequestInputAccess extends RemoteFlowSource::Range {
string attr_name;
RequestInputAccess() {
// attributes
this = flask::request().getMember(attr_name).getAnImmediateUse() and
attr_name in [
private class FlaskRequestAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// Methods
exists(string method_name | method_name in ["get_data", "get_json"] |
// Method access
nodeFrom = request().getAUse() and
nodeTo = request().getMember(method_name).getAnImmediateUse()
or
// Method call
nodeFrom = request().getMember(method_name).getAUse() and
nodeTo.(DataFlow::CallCfgNode).getFunction() = nodeFrom
)
or
// Attributes
nodeFrom = request().getAUse() and
exists(DataFlow::AttrRead read | nodeTo = read and read.getObject() = nodeFrom |
read.getAttributeName() in [
// str
"path", "full_path", "base_url", "url", "access_control_request_method",
"content_encoding", "content_md5", "content_type", "data", "method", "mimetype", "origin",
"query_string", "referrer", "remote_addr", "remote_user", "user_agent",
"content_encoding", "content_md5", "content_type", "data", "method", "mimetype",
"origin", "query_string", "referrer", "remote_addr", "remote_user", "user_agent",
// dict
"environ", "cookies", "mimetype_params", "view_args",
// json
@@ -371,34 +366,25 @@ private module FlaskModel {
// TODO: dict-like with wsgiref.headers.Header compatibility methods
"headers"
]
or
// methods (needs special handling to track bound-methods -- see `FlaskRequestMethodCallsAdditionalTaintStep` below)
this = FlaskRequestTracking::tainted_methods(attr_name).getAUse()
}
override string getSourceType() { result = "flask.request input" }
}
private class FlaskRequestMethodCallsAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// NOTE: `request -> request.tainted_method` part is handled as part of RequestInputAccess
// tainted_method -> tainted_method()
nodeFrom = FlaskRequestTracking::tainted_methods(_).getAUse() and
nodeTo.(DataFlow::CallCfgNode).getFunction() = nodeFrom
)
}
}
private class RequestInputMultiDict extends RequestInputAccess,
Werkzeug::werkzeug::datastructures::MultiDict::InstanceSource {
RequestInputMultiDict() { attr_name in ["args", "values", "form", "files"] }
private class RequestAttrMultiDict extends Werkzeug::werkzeug::datastructures::MultiDict::InstanceSource {
string attr_name;
RequestAttrMultiDict() {
attr_name in ["args", "values", "form", "files"] and
this = request().getMember(attr_name).getAnImmediateUse()
}
}
private class RequestInputFiles extends RequestInputMultiDict {
// TODO: Somehow specify that elements of `RequestInputFiles` are
private class RequestAttrFiles extends RequestAttrMultiDict {
// TODO: Somehow specify that elements of `RequestAttrFiles` are
// Werkzeug::werkzeug::datastructures::FileStorage and should have those additional taint steps
// AND that the 0-indexed argument to its' save method is a sink for path-injection.
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.FileStorage.save
RequestInputFiles() { attr_name = "files" }
RequestAttrFiles() { attr_name = "files" }
}
// ---------------------------------------------------------------------------