Python: Normalize additional taint steps for modeled classes

Such that it should be next to the other class-related predicates (such
as `instance()`), the class is called `AdditionalTaintStep`, and it
marked private.

I also moved any modeling of attributes as well, while I was at it.
This commit is contained in:
Rasmus Wriedt Larsen
2021-07-21 13:55:48 +02:00
parent be1cad864b
commit d2efe0b84d
8 changed files with 209 additions and 205 deletions

View File

@@ -293,6 +293,73 @@ module AiohttpWebModel {
/** Gets a reference to an instance of `aiohttp.web.Request`. */
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
/**
* Taint propagation for `aiohttp.web.Request`.
*
* See https://docs.aiohttp.org/en/stable/web_reference.html#request-and-base-request
*/
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// normal (non-async) methods
nodeFrom = Request::instance() and
nodeTo.(DataFlow::MethodCallNode).calls(nodeFrom, ["clone", "get_extra_info"])
or
// async methods
exists(DataFlow::MethodCallNode call, Await await |
nodeTo.asExpr() = await and
nodeFrom = Request::instance()
|
await.getValue() = any(DataFlow::Node awaitable | call.flowsTo(awaitable)).asExpr() and
call.calls(nodeFrom, ["read", "text", "json", "multipart", "post"])
)
or
// Attributes
nodeFrom = Request::instance() and
nodeTo.(DataFlow::AttrRead).getObject() = nodeFrom and
nodeTo.(DataFlow::AttrRead).getAttributeName() in [
"url", "rel_url", "forwarded", "host", "remote", "path", "path_qs", "raw_path", "query",
"headers", "transport", "cookies", "content", "_payload", "content_type", "charset",
"http_range", "if_modified_since", "if_unmodified_since", "if_range", "match_info"
]
}
}
/** An attribute read on an `aiohttp.web.Request` that is a `MultiDictProxy` instance. */
class AiohttpRequestMultiDictProxyInstances extends Multidict::MultiDictProxy::InstanceSource {
AiohttpRequestMultiDictProxyInstances() {
this.(DataFlow::AttrRead).getObject() = Request::instance() and
this.(DataFlow::AttrRead).getAttributeName() in ["query", "headers"]
or
// Handle the common case of `x = await request.post()`
// but don't try to handle anything else, since we don't have an easy way to do this yet.
// TODO: more complete handling of `await request.post()`
exists(Await await, DataFlow::CallCfgNode call, DataFlow::AttrRead read |
this.asExpr() = await
|
read.(DataFlow::AttrRead).getObject() = Request::instance() and
read.(DataFlow::AttrRead).getAttributeName() = "post" and
call.getFunction() = read and
await.getValue() = call.asExpr()
)
}
}
/** An attribute read on an `aiohttp.web.Request` that is a `yarl.URL` instance. */
class AiohttpRequestYarlUrlInstances extends Yarl::Url::InstanceSource {
AiohttpRequestYarlUrlInstances() {
this.(DataFlow::AttrRead).getObject() = Request::instance() and
this.(DataFlow::AttrRead).getAttributeName() in ["url", "rel_url"]
}
}
/** An attribute read on an `aiohttp.web.Request` that is a `aiohttp.StreamReader` instance. */
class AiohttpRequestStreamReaderInstances extends StreamReader::InstanceSource {
AiohttpRequestStreamReaderInstances() {
this.(DataFlow::AttrRead).getObject() = Request::instance() and
this.(DataFlow::AttrRead).getAttributeName() in ["content", "_payload"]
}
}
}
/**
@@ -357,7 +424,7 @@ module AiohttpWebModel {
/**
* Taint propagation for `aiohttp.StreamReader`.
*/
private class AiohttpStreamReaderAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// normal (non-async) methods
nodeFrom = instance() and
@@ -425,73 +492,6 @@ module AiohttpWebModel {
}
}
/**
* Taint propagation for `aiohttp.web.Request`.
*
* See https://docs.aiohttp.org/en/stable/web_reference.html#request-and-base-request
*/
private class AiohttpRequestAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// normal (non-async) methods
nodeFrom = Request::instance() and
nodeTo.(DataFlow::MethodCallNode).calls(nodeFrom, ["clone", "get_extra_info"])
or
// async methods
exists(DataFlow::MethodCallNode call, Await await |
nodeTo.asExpr() = await and
nodeFrom = Request::instance()
|
await.getValue() = any(DataFlow::Node awaitable | call.flowsTo(awaitable)).asExpr() and
call.calls(nodeFrom, ["read", "text", "json", "multipart", "post"])
)
or
// Attributes
nodeFrom = Request::instance() and
nodeTo.(DataFlow::AttrRead).getObject() = nodeFrom and
nodeTo.(DataFlow::AttrRead).getAttributeName() in [
"url", "rel_url", "forwarded", "host", "remote", "path", "path_qs", "raw_path", "query",
"headers", "transport", "cookies", "content", "_payload", "content_type", "charset",
"http_range", "if_modified_since", "if_unmodified_since", "if_range", "match_info"
]
}
}
/** An attribute read on an `aiohttp.web.Request` that is a `MultiDictProxy` instance. */
class AiohttpRequestMultiDictProxyInstances extends Multidict::MultiDictProxy::InstanceSource {
AiohttpRequestMultiDictProxyInstances() {
this.(DataFlow::AttrRead).getObject() = Request::instance() and
this.(DataFlow::AttrRead).getAttributeName() in ["query", "headers"]
or
// Handle the common case of `x = await request.post()`
// but don't try to handle anything else, since we don't have an easy way to do this yet.
// TODO: more complete handling of `await request.post()`
exists(Await await, DataFlow::CallCfgNode call, DataFlow::AttrRead read |
this.asExpr() = await
|
read.(DataFlow::AttrRead).getObject() = Request::instance() and
read.(DataFlow::AttrRead).getAttributeName() = "post" and
call.getFunction() = read and
await.getValue() = call.asExpr()
)
}
}
/** An attribute read on an `aiohttp.web.Request` that is a `yarl.URL` instance. */
class AiohttpRequestYarlUrlInstances extends Yarl::Url::InstanceSource {
AiohttpRequestYarlUrlInstances() {
this.(DataFlow::AttrRead).getObject() = Request::instance() and
this.(DataFlow::AttrRead).getAttributeName() in ["url", "rel_url"]
}
}
/** An attribute read on an `aiohttp.web.Request` that is a `aiohttp.StreamReader` instance. */
class AiohttpRequestStreamReaderInstances extends StreamReader::InstanceSource {
AiohttpRequestStreamReaderInstances() {
this.(DataFlow::AttrRead).getObject() = Request::instance() and
this.(DataFlow::AttrRead).getAttributeName() in ["content", "_payload"]
}
}
// ---------------------------------------------------------------------------
// aiohttp.web Response modeling
// ---------------------------------------------------------------------------

View File

@@ -340,7 +340,7 @@ private module Django {
/**
* Taint propagation for `django.utils.datastructures.MultiValueDict`.
*/
class MultiValueDictAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// class instantiation
exists(ClassInstantiation call |
@@ -388,7 +388,7 @@ private module Django {
/**
* Taint propagation for `django.core.files.uploadedfile.UploadedFile`.
*/
class UploadedFileAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// Attributes
nodeFrom = instance() and
@@ -436,7 +436,7 @@ private module Django {
/**
* Taint propagation for `django.urls.ResolverMatch`.
*/
class ResolverMatchAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// Attributes
nodeFrom = instance() and
@@ -743,6 +743,111 @@ private module PrivateDjango {
/** Gets a reference to an instance of `django.http.request.HttpRequest`. */
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
/**
* Taint propagation for `django.http.request.HttpRequest`.
*/
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// normal (non-async) methods
nodeFrom = django::http::request::HttpRequest::instance() and
nodeTo
.(DataFlow::MethodCallNode)
.calls(nodeFrom,
["get_full_path", "get_full_path_info", "read", "readline", "readlines"])
or
// special handling of the `build_absolute_uri` method, see
// https://docs.djangoproject.com/en/3.0/ref/request-response/#django.http.HttpRequest.build_absolute_uri
exists(DataFlow::AttrRead attr, DataFlow::CallCfgNode call, DataFlow::Node instance |
instance = django::http::request::HttpRequest::instance() and
attr.getObject() = instance
|
attr.getAttributeName() = "build_absolute_uri" and
nodeTo.(DataFlow::CallCfgNode).getFunction() = attr and
call = nodeTo and
(
not exists(call.getArg(_)) and
not exists(call.getArgByName(_)) and
nodeFrom = instance
or
nodeFrom = call.getArg(0)
or
nodeFrom = call.getArgByName("location")
)
)
or
// Attributes
nodeFrom = django::http::request::HttpRequest::instance() and
nodeTo.(DataFlow::AttrRead).getObject() = nodeFrom and
nodeTo.(DataFlow::AttrRead).getAttributeName() in [
// str / bytes
"body", "path", "path_info", "method", "encoding", "content_type",
// django.http.QueryDict
"GET", "POST",
// dict[str, str]
"content_params", "COOKIES",
// dict[str, Any]
"META",
// HttpHeaders (case insensitive dict-like)
"headers",
// MultiValueDict[str, UploadedFile]
"FILES",
// django.urls.ResolverMatch
"resolver_match"
]
// TODO: Handle that a HttpRequest is iterable
}
}
/** An attribute read on an django request that is a `MultiValueDict` instance. */
private class DjangoHttpRequestMultiValueDictInstances extends Django::MultiValueDict::InstanceSource {
DjangoHttpRequestMultiValueDictInstances() {
this.(DataFlow::AttrRead).getObject() = instance() and
this.(DataFlow::AttrRead).getAttributeName() in ["GET", "POST", "FILES"]
}
}
/** An attribute read on an django request that is a `ResolverMatch` instance. */
private class DjangoHttpRequestResolverMatchInstances extends Django::ResolverMatch::InstanceSource {
DjangoHttpRequestResolverMatchInstances() {
this.(DataFlow::AttrRead).getObject() = instance() and
this.(DataFlow::AttrRead).getAttributeName() = "resolver_match"
}
}
/** An `UploadedFile` instance that originates from a django request. */
private class DjangoHttpRequestUploadedFileInstances extends Django::UploadedFile::InstanceSource {
DjangoHttpRequestUploadedFileInstances() {
// TODO: this currently only works in local-scope, since writing type-trackers for
// this is a little too much effort. Once API-graphs are available for more
// things, we can rewrite this.
//
// TODO: This approach for identifying member-access is very adhoc, and we should
// be able to do something more structured for providing modeling of the members
// of a container-object.
//
// dicts
exists(DataFlow::AttrRead files, DataFlow::Node dict |
files.accesses(instance(), "FILES") and
(
dict = files
or
dict.(DataFlow::MethodCallNode).calls(files, "dict")
)
|
this.asCfgNode().(SubscriptNode).getObject() = dict.asCfgNode()
or
this.(DataFlow::MethodCallNode).calls(dict, "get")
)
or
// getlist
exists(DataFlow::AttrRead files, DataFlow::MethodCallNode getlistCall |
files.accesses(instance(), "FILES") and
getlistCall.calls(files, "getlist") and
this.asCfgNode().(SubscriptNode).getObject() = getlistCall.asCfgNode()
)
}
}
}
}
@@ -2036,107 +2141,6 @@ private module PrivateDjango {
}
}
private class DjangoHttpRequestAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// normal (non-async) methods
nodeFrom = django::http::request::HttpRequest::instance() and
nodeTo
.(DataFlow::MethodCallNode)
.calls(nodeFrom, ["get_full_path", "get_full_path_info", "read", "readline", "readlines"])
or
// special handling of the `build_absolute_uri` method, see
// https://docs.djangoproject.com/en/3.0/ref/request-response/#django.http.HttpRequest.build_absolute_uri
exists(DataFlow::AttrRead attr, DataFlow::CallCfgNode call, DataFlow::Node instance |
instance = django::http::request::HttpRequest::instance() and
attr.getObject() = instance
|
attr.getAttributeName() = "build_absolute_uri" and
nodeTo.(DataFlow::CallCfgNode).getFunction() = attr and
call = nodeTo and
(
not exists(call.getArg(_)) and
not exists(call.getArgByName(_)) and
nodeFrom = instance
or
nodeFrom = call.getArg(0)
or
nodeFrom = call.getArgByName("location")
)
)
or
// Attributes
nodeFrom = django::http::request::HttpRequest::instance() and
nodeTo.(DataFlow::AttrRead).getObject() = nodeFrom and
nodeTo.(DataFlow::AttrRead).getAttributeName() in [
// str / bytes
"body", "path", "path_info", "method", "encoding", "content_type",
// django.http.QueryDict
"GET", "POST",
// dict[str, str]
"content_params", "COOKIES",
// dict[str, Any]
"META",
// HttpHeaders (case insensitive dict-like)
"headers",
// MultiValueDict[str, UploadedFile]
"FILES",
// django.urls.ResolverMatch
"resolver_match"
]
// TODO: Handle that a HttpRequest is iterable
}
}
/** An attribute read on an django request that is a `MultiValueDict` instance. */
class DjangoHttpRequestMultiValueDictInstances extends Django::MultiValueDict::InstanceSource {
DjangoHttpRequestMultiValueDictInstances() {
this.(DataFlow::AttrRead).getObject() = django::http::request::HttpRequest::instance() and
this.(DataFlow::AttrRead).getAttributeName() in ["GET", "POST", "FILES"]
}
}
/** An attribute read on an django request that is a `ResolverMatch` instance. */
class DjangoHttpRequestResolverMatchInstances extends Django::ResolverMatch::InstanceSource {
DjangoHttpRequestResolverMatchInstances() {
this.(DataFlow::AttrRead).getObject() = django::http::request::HttpRequest::instance() and
this.(DataFlow::AttrRead).getAttributeName() = "resolver_match"
}
}
/** An `UploadedFile` instance that originates from a django request. */
class DjangoHttpRequestUploadedFileInstances extends Django::UploadedFile::InstanceSource {
DjangoHttpRequestUploadedFileInstances() {
// TODO: this currently only works in local-scope, since writing type-trackers for
// this is a little too much effort. Once API-graphs are available for more
// things, we can rewrite this.
//
// TODO: This approach for identifying member-access is very adhoc, and we should
// be able to do something more structured for providing modeling of the members
// of a container-object.
//
// dicts
exists(DataFlow::AttrRead files, DataFlow::Node dict |
files.accesses(django::http::request::HttpRequest::instance(), "FILES") and
(
dict = files
or
dict.(DataFlow::MethodCallNode).calls(files, "dict")
)
|
this.asCfgNode().(SubscriptNode).getObject() = dict.asCfgNode()
or
this.(DataFlow::MethodCallNode).calls(dict, "get")
)
or
// getlist
exists(DataFlow::AttrRead files, DataFlow::MethodCallNode getlistCall |
files.accesses(django::http::request::HttpRequest::instance(), "FILES") and
getlistCall.calls(files, "getlist") and
this.asCfgNode().(SubscriptNode).getObject() = getlistCall.asCfgNode()
)
}
}
// ---------------------------------------------------------------------------
// django.shortcuts.redirect
// ---------------------------------------------------------------------------

View File

@@ -82,7 +82,7 @@ private module MarkupSafeModel {
}
/** Taint propagation for `markupsafe.Markup`. */
class AddtionalTaintSteps extends TaintTracking::AdditionalTaintStep {
private class AddtionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
nodeTo.(ClassInstantiation).getArg(0) = nodeFrom
}

View File

@@ -63,7 +63,7 @@ module Multidict {
*
* See https://multidict.readthedocs.io/en/stable/multidict.html#multidictproxy
*/
class MultiDictProxyAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// class instantiation
exists(ClassInstantiation call |

View File

@@ -47,7 +47,7 @@ module Stdlib {
/**
* Taint propagation for file-like objects.
*/
class FileLikeObjectAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// result of method call is tainted
nodeFrom = instance() and

View File

@@ -110,6 +110,31 @@ private module Twisted {
/** Gets a reference to an instance of `twisted.web.server.Request`. */
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
/**
* Taint propagation for `twisted.web.server.Request`.
*/
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// normal (non-async) methods
nodeFrom = instance() and
nodeTo
.(DataFlow::MethodCallNode)
.calls(nodeFrom,
[
"getCookie", "getHeader", "getAllHeaders", "getUser", "getPassword", "getHost",
"getRequestHostname"
])
or
// Attributes
nodeFrom = instance() and
nodeTo.(DataFlow::AttrRead).getObject() = nodeFrom and
nodeTo.(DataFlow::AttrRead).getAttributeName() in [
"uri", "path", "prepath", "postpath", "content", "args", "received_cookies",
"requestHeaders", "user", "password", "host"
]
}
}
}
/**
@@ -125,31 +150,6 @@ private module Twisted {
override string getSourceType() { result = "twisted.web.server.Request" }
}
/**
* Taint propagation for `twisted.web.server.Request`.
*/
private class TwistedRequestAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// normal (non-async) methods
nodeFrom = Request::instance() and
nodeTo
.(DataFlow::MethodCallNode)
.calls(nodeFrom,
[
"getCookie", "getHeader", "getAllHeaders", "getUser", "getPassword", "getHost",
"getRequestHostname"
])
or
// Attributes
nodeFrom = Request::instance() and
nodeTo.(DataFlow::AttrRead).getObject() = nodeFrom and
nodeTo.(DataFlow::AttrRead).getAttributeName() in [
"uri", "path", "prepath", "postpath", "content", "args", "received_cookies",
"requestHeaders", "user", "password", "host"
]
}
}
/**
* A parameter of a request handler method (on a `twisted.web.resource.Resource` subclass)
* that is also given remote user input. (a bit like RoutedParameter).

View File

@@ -47,7 +47,7 @@ module Werkzeug {
/** Gets a reference to an instance of `werkzeug.datastructures.MultiDict`. */
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
private class MultiDictAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
nodeFrom = instance() and
nodeTo.(DataFlow::MethodCallNode).calls(nodeFrom, "getlist")
@@ -87,7 +87,7 @@ module Werkzeug {
/** Gets a reference to an instance of `werkzeug.datastructures.FileStorage`. */
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
private class FileStorageAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
nodeFrom = instance() and
exists(DataFlow::AttrRead read | nodeTo = read |
@@ -152,7 +152,7 @@ module Werkzeug {
/**
* Taint propagation for `werkzeug.datastructures.Headers`.
*/
class HeadersAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// normal (non-async) methods
nodeFrom = instance() and
@@ -194,7 +194,7 @@ module Werkzeug {
/**
* Taint propagation for `werkzeug.datastructures.Authorization`.
*/
class AuthorizationAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// Attributes
nodeFrom = instance() and

View File

@@ -55,7 +55,7 @@ module Yarl {
*
* See https://yarl.readthedocs.io/en/stable/api.html#yarl.URL
*/
class YarlUrlAdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
private class AdditionalTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// class instantiation
exists(ClassInstantiation call |