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

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

@@ -74,14 +74,8 @@ private module FlaskModel {
API::Node instance() { result = classRef().getReturn() } API::Node instance() { result = classRef().getReturn() }
} }
// --------------------------------------------------------------------------- /** Gets a reference to the `flask.request` object. */
// flask API::Node request() { result = API::moduleImport("flask").getMember("request") }
// ---------------------------------------------------------------------------
/** 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 * Provides models for the `flask.Response` class
@@ -305,100 +299,92 @@ private module FlaskModel {
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// flask.Request taint modeling // flask.Request taint modeling
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// TODO: Do we even need this class? :|
/** /**
* A source of remote flow from a flask request. * A source of remote flow from a flask request.
* *
* See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Request * See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Request
*/ */
private class RequestSource extends RemoteFlowSource::Range { private class RequestSource extends RemoteFlowSource::Range {
RequestSource() { this = flask::request().getAUse() } RequestSource() { this = request().getAUse() }
override string getSourceType() { result = "flask.request" } 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 * See https://flask.palletsprojects.com/en/1.1.x/api/#flask.Request
*/ */
private class RequestInputAccess extends RemoteFlowSource::Range { 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",
// dict
"environ", "cookies", "mimetype_params", "view_args",
// json
"json",
// List[str]
"access_route",
// file-like
"stream", "input_stream",
// MultiDict[str, str]
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.MultiDict
"args", "values", "form",
// MultiDict[str, FileStorage]
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.FileStorage
// TODO: FileStorage needs extra taint steps
"files",
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.HeaderSet
"access_control_request_headers", "pragma",
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.Accept
// TODO: Kinda badly modeled for now -- has type List[Tuple[value, quality]], and some extra methods
"accept_charsets", "accept_encodings", "accept_languages", "accept_mimetypes",
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.Authorization
// TODO: dict subclass with extra attributes like `username` and `password`
"authorization",
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.RequestCacheControl
// TODO: has attributes like `no_cache`, and `to_header` method (actually, many of these models do)
"cache_control",
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.Headers
// TODO: dict-like with wsgiref.headers.Header compatibility methods
"headers"
]
)
}
}
private class RequestAttrMultiDict extends Werkzeug::werkzeug::datastructures::MultiDict::InstanceSource {
string attr_name; string attr_name;
RequestInputAccess() { RequestAttrMultiDict() {
// attributes attr_name in ["args", "values", "form", "files"] and
this = flask::request().getMember(attr_name).getAnImmediateUse() and this = request().getMember(attr_name).getAnImmediateUse()
attr_name 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",
// dict
"environ", "cookies", "mimetype_params", "view_args",
// json
"json",
// List[str]
"access_route",
// file-like
"stream", "input_stream",
// MultiDict[str, str]
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.MultiDict
"args", "values", "form",
// MultiDict[str, FileStorage]
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.FileStorage
// TODO: FileStorage needs extra taint steps
"files",
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.HeaderSet
"access_control_request_headers", "pragma",
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.Accept
// TODO: Kinda badly modeled for now -- has type List[Tuple[value, quality]], and some extra methods
"accept_charsets", "accept_encodings", "accept_languages", "accept_mimetypes",
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.Authorization
// TODO: dict subclass with extra attributes like `username` and `password`
"authorization",
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.RequestCacheControl
// TODO: has attributes like `no_cache`, and `to_header` method (actually, many of these models do)
"cache_control",
// https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.Headers
// 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, private class RequestAttrFiles extends RequestAttrMultiDict {
Werkzeug::werkzeug::datastructures::MultiDict::InstanceSource { // TODO: Somehow specify that elements of `RequestAttrFiles` are
RequestInputMultiDict() { attr_name in ["args", "values", "form", "files"] }
}
private class RequestInputFiles extends RequestInputMultiDict {
// TODO: Somehow specify that elements of `RequestInputFiles` are
// Werkzeug::werkzeug::datastructures::FileStorage and should have those additional taint steps // 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. // 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 // https://werkzeug.palletsprojects.com/en/1.0.x/datastructures/#werkzeug.datastructures.FileStorage.save
RequestInputFiles() { attr_name = "files" } RequestAttrFiles() { attr_name = "files" }
} }
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------