-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Python: Model the Pyramid framework #16300
Changes from 4 commits
88e3227
f85ee38
86d1e5b
2a04598
7df8b1b
ba054bd
4f22b91
c6372d5
fd55713
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,234 @@ | ||
/** | ||
* Provides classes modeling security-relevant aspects of the `pyramid` PyPI package. | ||
* See https://docs.pylonsproject.org/projects/pyramid/. | ||
*/ | ||
|
||
private import python | ||
private import semmle.python.dataflow.new.RemoteFlowSources | ||
private import semmle.python.dataflow.new.TaintTracking | ||
private import semmle.python.Concepts | ||
private import semmle.python.ApiGraphs | ||
private import semmle.python.dataflow.new.FlowSummary | ||
private import semmle.python.frameworks.internal.PoorMansFunctionResolution | ||
private import semmle.python.frameworks.internal.InstanceTaintStepsHelper | ||
private import semmle.python.frameworks.data.ModelsAsData | ||
private import semmle.python.frameworks.Stdlib | ||
|
||
/** | ||
* Provides models for the `pyramid` PyPI package. | ||
* See https://docs.pylonsproject.org/projects/pyramid/. | ||
*/ | ||
module Pyramid { | ||
// TODO: qldoc | ||
module View { | ||
/** | ||
* A callable that could be used as a pyramid view callable. | ||
*/ | ||
private class PotentialViewCallable extends Function { | ||
PotentialViewCallable() { | ||
this.getPositionalParameterCount() = 1 and | ||
this.getArgName(0) = "request" | ||
or | ||
this.getPositionalParameterCount() = 2 and | ||
this.getArgName(0) = "context" and | ||
this.getArgName(1) = "request" | ||
} | ||
|
||
/** Gets the `request` parameter of this view callable. */ | ||
Parameter getRequestParameter() { result = this.getArgByName("request") } | ||
} | ||
|
||
abstract class ViewCallable extends PotentialViewCallable, Http::Server::RequestHandler::Range { | ||
override Parameter getARoutedParameter() { result = this.getRequestParameter() } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for a route like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think pyramid has parameters like that (instead There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes please 👍 |
||
|
||
override string getFramework() { result = "Pyramid" } | ||
} | ||
|
||
private class ViewCallableFromDecorator extends ViewCallable { | ||
ViewCallableFromDecorator() { | ||
this.getADecorator() = | ||
API::moduleImport("pyramid") | ||
.getMember("view") | ||
.getMember("view_config") | ||
.getACall() | ||
.asExpr() | ||
} | ||
} | ||
|
||
private class ViewCallableFromConfigurator extends ViewCallable { | ||
ViewCallableFromConfigurator() { | ||
any(Configurator::AddViewCall c).getViewArg() = poorMansFunctionTracker(this) | ||
} | ||
} | ||
} | ||
|
||
module Configurator { | ||
/** Gets a reference to the class `pyramid.config.Configurator`. */ | ||
API::Node classRef() { | ||
result = API::moduleImport("pyramid").getMember("config").getMember("Configurator") | ||
} | ||
|
||
/** Gets a reference to an instance of `pyramid.config.Configurator`. */ | ||
private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) { | ||
t.start() and | ||
result = classRef().getACall() | ||
or | ||
exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t)) | ||
} | ||
|
||
/** Gets a reference to an instance of `pyramid.config.Configurator`. */ | ||
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) } | ||
|
||
class AddViewCall extends DataFlow::MethodCallNode { | ||
AddViewCall() { this.calls(instance(), "add_view") } | ||
|
||
DataFlow::Node getViewArg() { result = [this.getArg(0), this.getArgByName("view")] } | ||
} | ||
} | ||
|
||
module Request { | ||
abstract class InstanceSource extends DataFlow::LocalSourceNode { } | ||
|
||
/** Gets a reference to an instance of `pyramid.request.Request`. */ | ||
private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) { | ||
t.start() and | ||
result instanceof InstanceSource | ||
or | ||
exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t)) | ||
} | ||
|
||
/** Gets a reference to an instance of `pyramid.request.Request`. */ | ||
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) } | ||
|
||
private class RequestParameter extends InstanceSource, DataFlow::ParameterNode { | ||
RequestParameter() { this.getParameter() = any(View::ViewCallable vc).getRequestParameter() } | ||
} | ||
|
||
private class InstanceTaintSteps extends InstanceTaintStepsHelper { | ||
InstanceTaintSteps() { this = "pyramid.request.Request" } | ||
|
||
override DataFlow::Node getInstance() { result = instance() } | ||
|
||
override string getAttributeName() { | ||
result in [ | ||
"accept", "accept_charset", "accept_encoding", "accept_language", "application_url", | ||
"as_bytes", "authorization", "body", "body_file", "body_file_raw", "body_file_seekable", | ||
"cache_control", "client_addr", "content_type", "cookies", "domain", "headers", "host", | ||
"host_port", "host_url", "GET", "if_match", "if_none_match", "if_range", | ||
"if_none_match", "json", "json_body", "params", "path", "path_info", "path_qs", | ||
"path_url", "POST", "pragma", "query_string", "range", "referer", "referrer", "text", | ||
"url", "urlargs", "urlvars", "user_agent" | ||
] | ||
} | ||
|
||
override string getMethodName() { | ||
result in ["as_bytes", "copy", "copy_get", "path_info_peek", "path_info_pop"] | ||
} | ||
|
||
override string getAsyncMethodName() { none() } | ||
} | ||
|
||
private class RequestCopyCall extends InstanceSource, DataFlow::MethodCallNode { | ||
RequestCopyCall() { this.calls(instance(), ["copy", "copy_get"]) } | ||
} | ||
|
||
private class RequestBodyFileLike extends Stdlib::FileLikeObject::InstanceSource instanceof DataFlow::AttrRead | ||
{ | ||
RequestBodyFileLike() { | ||
this.getObject() = instance() and | ||
this.getAttributeName() = ["body_file", "body_file_raw", "body_file_seekable"] | ||
} | ||
} | ||
} | ||
|
||
module Response { | ||
private class PyramidReturnResponse extends Http::Server::HttpResponse::Range { | ||
PyramidReturnResponse() { | ||
this.asCfgNode() = any(View::ViewCallable vc).getAReturnValueFlowNode() and | ||
not this = instance() | ||
} | ||
|
||
override DataFlow::Node getBody() { result = this } | ||
|
||
override DataFlow::Node getMimetypeOrContentTypeArg() { none() } | ||
|
||
override string getMimetypeDefault() { result = "text/html" } | ||
} | ||
|
||
private API::Node classRef() { | ||
result = API::moduleImport("pyramid").getMember("response").getMember("Response") | ||
} | ||
|
||
abstract class InstanceSource extends DataFlow::LocalSourceNode, | ||
Http::Server::HttpResponse::Range | ||
{ } | ||
|
||
/** Gets a reference to an instance of `pyramid.request.Request`. */ | ||
private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) { | ||
t.start() and | ||
result instanceof InstanceSource | ||
or | ||
exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t)) | ||
} | ||
|
||
/** Gets a reference to an instance of `pyramid.request.Request`. */ | ||
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) } | ||
|
||
private class ClassInstantiation extends InstanceSource, DataFlow::CallCfgNode { | ||
ClassInstantiation() { this = classRef().getACall() } | ||
|
||
override DataFlow::Node getBody() { result = [this.getArg(0), this.getArgByName("body")] } | ||
|
||
override DataFlow::Node getMimetypeOrContentTypeArg() { | ||
result = [this.getArg(4), this.getArgByName("content_type")] | ||
} | ||
|
||
override string getMimetypeDefault() { result = "text/html" } | ||
} | ||
|
||
private class ResponseBodySet extends Http::Server::HttpResponse::Range instanceof DataFlow::AttrWrite | ||
{ | ||
string attrName; | ||
|
||
ResponseBodySet() { | ||
this.getObject() = instance() and | ||
this.getAttributeName() = attrName and | ||
attrName in ["body", "body_file", "json", "json_body", "text", "ubody", "unicode_body"] | ||
} | ||
|
||
override DataFlow::Node getBody() { result = this.(DataFlow::AttrWrite).getValue() } | ||
|
||
override DataFlow::Node getMimetypeOrContentTypeArg() { none() } | ||
|
||
override string getMimetypeDefault() { | ||
if attrName in ["json", "json_body"] | ||
then result = "application/json" | ||
else result = "text/html" | ||
} | ||
} | ||
|
||
private class RequestResponseAttr extends InstanceSource instanceof DataFlow::AttrRead { | ||
RequestResponseAttr() { | ||
this.getObject() = Request::instance() and this.getAttributeName() = "response" | ||
} | ||
|
||
override DataFlow::Node getBody() { none() } | ||
|
||
override DataFlow::Node getMimetypeOrContentTypeArg() { none() } | ||
|
||
override string getMimetypeDefault() { result = "text/html" } | ||
} | ||
|
||
private class SetCookieCall extends Http::Server::CookieWrite::Range, DataFlow::MethodCallNode { | ||
SetCookieCall() { this.calls(instance(), "set_cookie") } | ||
|
||
override DataFlow::Node getHeaderArg() { none() } | ||
|
||
override DataFlow::Node getNameArg() { result = [this.getArg(0), this.getArgByName("name")] } | ||
|
||
override DataFlow::Node getValueArg() { | ||
result = [this.getArg(1), this.getArgByName("value")] | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
testFailures | ||
failures |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
import python | ||
import experimental.meta.ConceptsTest |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
argumentToEnsureNotTaintedNotMarkedAsSpurious | ||
untaintedArgumentToEnsureTaintedNotMarkedAsMissing | ||
testFailures | ||
failures |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
import experimental.meta.InlineTaintTest | ||
import MakeInlineTaintTest<TestTaintTrackingConfig> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
from pyramid.view import view_config | ||
from pyramid.config import Configurator | ||
from pyramid.response import Response | ||
from wsgiref.simple_server import make_server | ||
|
||
def ignore(*args, **kwargs): pass | ||
ensure_tainted = ensure_not_tainted = ignore | ||
|
||
@view_config(route_name="test1") | ||
def test1(request): # $ requestHandler routedParameter=request | ||
ensure_tainted( | ||
request, # $ tainted | ||
|
||
request.accept, # $ tainted | ||
request.accept_charset, # $ tainted | ||
request.accept_encoding, # $ tainted | ||
request.accept_language, # $ tainted | ||
request.authorization, # $ tainted | ||
request.cache_control, # $ tainted | ||
request.client_addr, # $ tainted | ||
request.content_type, # $ tainted | ||
request.domain, # $ tainted | ||
request.host, # $ tainted | ||
request.host_port, # $ tainted | ||
request.host_url, # $ tainted | ||
request.if_match, # $ tainted | ||
request.if_none_match, # $ tainted | ||
request.if_range, # $ tainted | ||
request.pragma, # $ tainted | ||
request.range, # $ tainted | ||
request.referer, # $ tainted | ||
request.referrer, # $ tainted | ||
request.user_agent, # $ tainted | ||
|
||
request.as_bytes, # $ tainted | ||
|
||
request.body, # $ tainted | ||
request.body_file.read(), # $ tainted | ||
request.body_file_raw.read(), # $ tainted | ||
request.body_file_seekable.read(),# $ tainted | ||
|
||
request.json, # $ tainted | ||
request.json_body, # $ tainted | ||
request.json['a']['b'][0]['c'], # $ tainted | ||
|
||
request.text, # $ tainted | ||
|
||
request.path, # $ tainted | ||
request.path_info, # $ tainted | ||
request.path_info_peek(), # $ tainted | ||
request.path_info_pop(), # $ tainted | ||
request.path_qs, # $ tainted | ||
request.path_url, # $ tainted | ||
request.query_string, # $ tainted | ||
|
||
request.url, # $ tainted | ||
request.urlargs, # $ tainted | ||
request.urlvars, # $ tainted | ||
|
||
request.GET['a'], # $ tainted | ||
request.POST['b'], # $ tainted | ||
request.cookies['c'], # $ tainted | ||
request.params['d'], # $ tainted | ||
request.headers['X-My-Header'], # $ tainted | ||
request.GET.values(), # $ tainted | ||
|
||
request.copy(), # $ tainted | ||
request.copy_get(), # $ tainted | ||
request.copy().GET['a'], # $ tainted | ||
request.copy_get().body # $ tainted | ||
) | ||
|
||
return Response("Ok") # $ HttpResponse responseBody="Ok" mimetype=text/html | ||
|
||
def test2(request): # $ requestHandler routedParameter=request | ||
ensure_tainted(request) # $ tainted | ||
|
||
resp = Response("Ok", content_type="text/plain") # $ HttpResponse responseBody="Ok" mimetype=text/plain | ||
resp.body = "Ok2" # $ HttpResponse responseBody="Ok2" SPURIOUS: mimetype=text/html | ||
return resp | ||
|
||
@view_config(route_name="test3", renderer="string") | ||
def test3(context, request): # $ requestHandler routedParameter=request | ||
ensure_tainted(request) # $ tainted | ||
resp = request.response # $ HttpResponse mimetype=text/html | ||
resp.set_cookie("hi", "there") # $ CookieWrite CookieName="hi" CookieValue="there" | ||
resp.set_cookie(value="there", name="hi") # $ CookieWrite CookieName="hi" CookieValue="there" | ||
return "Ok" # $ HttpResponse responseBody="Ok" mimetype=text/html | ||
|
||
if __name__ == "__main__": | ||
with Configurator() as config: | ||
for i in range(1,4): | ||
config.add_route(f"test{i}", f"/test{i}") | ||
config.add_view(test2, route_name="test2") | ||
config.scan() | ||
server = make_server('127.0.0.1', 8000, config.make_wsgi_app()) | ||
print("serving") | ||
server.serve_forever() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a hard requirement that the request parameter MUST be called
request
? or could it be namedreq
as well?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation suggests that it should:
however empirical testing seems to show it can allow other names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, let's include that in our tests, and let's change our modeling to handle that 👍