Merge pull request #2140 from RasmusWL/python-fix-flask

Python: Modernise flask + correctly handle flask.make_response
This commit is contained in:
Taus
2019-11-15 14:55:27 +01:00
committed by GitHub
23 changed files with 244 additions and 161 deletions

View File

@@ -17,7 +17,7 @@ import semmle.python.web.flask.General
from CallNode call, Object isTrue
where
call = theFlaskClass().declaredAttribute("run").(FunctionObject).getACall() and
call = theFlaskClass().declaredAttribute("run").(FunctionValue).getACall() and
call.getArgByName("debug").refersTo(isTrue) and
isTrue.booleanValue() = true
select call, "A Flask app appears to be run in debug mode. This may allow an attacker to run arbitrary code through the debugger."

View File

@@ -1,31 +1,22 @@
import python
import semmle.python.web.Http
/** The flask module */
ModuleObject theFlaskModule() {
result = ModuleObject::named("flask")
}
import semmle.python.web.flask.Response
/** The flask app class */
ClassObject theFlaskClass() {
result = theFlaskModule().attr("Flask")
}
ClassValue theFlaskClass() { result = Value::named("flask.Flask") }
/** The flask MethodView class */
ClassObject theFlaskMethodViewClass() {
result = ModuleObject::named("flask.views").attr("MethodView")
}
ClassValue theFlaskMethodViewClass() { result = Value::named("flask.views.MethodView") }
ClassObject theFlaskReponseClass() {
result = theFlaskModule().attr("Response")
}
ClassValue theFlaskReponseClass() { result = Value::named("flask.Response") }
/** Holds if `route` is routed to `func`
/**
* Holds if `route` is routed to `func`
* by decorating `func` with `app.route(route)`
*/
predicate app_route(ControlFlowNode route, Function func) {
exists(CallNode route_call, CallNode decorator_call |
route_call.getFunction().(AttrNode).getObject("route").refersTo(_, theFlaskClass(), _) and
route_call.getFunction().(AttrNode).getObject("route").pointsTo().getClass() = theFlaskClass() and
decorator_call.getFunction() = route_call and
route_call.getArg(0) = route and
decorator_call.getArg(0).getNode().(FunctionExpr).getInnerScope() = func
@@ -35,8 +26,9 @@ predicate app_route(ControlFlowNode route, Function func) {
/* Helper for add_url_rule */
private predicate add_url_rule_call(ControlFlowNode regex, ControlFlowNode callable) {
exists(CallNode call |
call.getFunction().(AttrNode).getObject("add_url_rule").refersTo(_, theFlaskClass(), _) and
regex = call.getArg(0) |
call.getFunction().(AttrNode).getObject("add_url_rule").pointsTo().getClass() = theFlaskClass() and
regex = call.getArg(0)
|
callable = call.getArg(2) or
callable = call.getArgByName("view_func")
)
@@ -44,21 +36,19 @@ private predicate add_url_rule_call(ControlFlowNode regex, ControlFlowNode calla
/** Holds if urls matching `regex` are routed to `func` */
predicate add_url_rule(ControlFlowNode regex, Function func) {
exists(ControlFlowNode callable |
add_url_rule_call(regex, callable)
|
exists(PyFunctionObject f | f.getFunction() = func and callable.refersTo(f))
exists(ControlFlowNode callable | add_url_rule_call(regex, callable) |
exists(PythonFunctionValue f | f.getScope() = func and callable.pointsTo(f))
or
/* MethodView.as_view() */
exists(MethodViewClass view_cls |
view_cls.asTaint().taints(callable) |
func = view_cls.lookupAttribute(httpVerbLower()).(FunctionObject).getFunction()
exists(MethodViewClass view_cls | view_cls.asTaint().taints(callable) |
func = view_cls.lookup(httpVerbLower()).(FunctionValue).getScope()
)
/* TO DO -- Handle Views that aren't MethodViews */
/* TODO: -- Handle Views that aren't MethodViews */
)
}
/** Holds if urls matching `regex` are routed to `func` using
/**
* Holds if urls matching `regex` are routed to `func` using
* any of flask's routing mechanisms.
*/
predicate flask_routing(ControlFlowNode regex, Function func) {
@@ -68,58 +58,42 @@ predicate flask_routing(ControlFlowNode regex, Function func) {
}
/** A class that extends flask.views.MethodView */
private class MethodViewClass extends ClassObject {
MethodViewClass() {
this.getAnImproperSuperType() = theFlaskMethodViewClass()
}
private class MethodViewClass extends ClassValue {
MethodViewClass() { this.getASuperType() = theFlaskMethodViewClass() }
/* As we are restricted to strings for taint kinds, we need to map these classes to strings. */
string taintString() {
result = "flask/" + this.getQualifiedName() + ".as.view"
}
string taintString() { result = "flask/" + this.getQualifiedName() + ".as.view" }
/* As we are restricted to strings for taint kinds, we need to map these classes to strings. */
TaintKind asTaint() {
result = this.taintString()
}
TaintKind asTaint() { result = this.taintString() }
}
private class MethodViewTaint extends TaintKind {
MethodViewTaint() {
any(MethodViewClass cls).taintString() = this
}
MethodViewTaint() { any(MethodViewClass cls).taintString() = this }
}
/** A source of method view "taint"s. */
private class AsView extends TaintSource {
AsView() {
exists(ClassObject view_class |
view_class.getAnImproperSuperType() = theFlaskMethodViewClass() and
this.(CallNode).getFunction().(AttrNode).getObject("as_view").refersTo(view_class)
exists(ClassValue view_class |
view_class.getASuperType() = theFlaskMethodViewClass() and
this.(CallNode).getFunction().(AttrNode).getObject("as_view").pointsTo(view_class)
)
}
override string toString() {
result = "flask.MethodView.as_view()"
}
override string toString() { result = "flask.MethodView.as_view()" }
override predicate isSourceOf(TaintKind kind) {
exists(MethodViewClass view_class |
kind = view_class.asTaint() and
this.(CallNode).getFunction().(AttrNode).getObject("as_view").refersTo(view_class)
this.(CallNode).getFunction().(AttrNode).getObject("as_view").pointsTo(view_class)
)
}
}
class FlaskCookieSet extends CookieSet, CallNode {
FlaskCookieSet() {
this.getFunction().(AttrNode).getObject("set_cookie").refersTo(_, theFlaskReponseClass(), _)
any(FlaskResponseTaintKind t).taints(this.getFunction().(AttrNode).getObject("set_cookie"))
}
override string toString() { result = CallNode.super.toString() }
@@ -127,6 +101,4 @@ class FlaskCookieSet extends CookieSet, CallNode {
override ControlFlowNode getKey() { result = this.getArg(0) }
override ControlFlowNode getValue() { result = this.getArg(1) }
}

View File

@@ -1,25 +1,21 @@
/** Provides class representing the `flask.redirect` function.
/**
* Provides class representing the `flask.redirect` function.
* This module is intended to be imported into a taint-tracking query
* to extend `TaintSink`.
*/
import python
import python
import semmle.python.security.TaintTracking
import semmle.python.security.strings.Basic
import semmle.python.web.flask.General
FunctionObject flask_redirect() {
result = theFlaskModule().attr("redirect")
}
FunctionValue flask_redirect() { result = Value::named("flask.redirect") }
/**
* Represents an argument to the `flask.redirect` function.
*/
class FlaskRedirect extends HttpRedirectTaintSink {
override string toString() {
result = "flask.redirect"
}
override string toString() { result = "flask.redirect" }
FlaskRedirect() {
exists(CallNode call |
@@ -27,5 +23,4 @@ class FlaskRedirect extends HttpRedirectTaintSink {
this = call.getAnArg()
)
}
}

View File

@@ -1,79 +1,56 @@
import python
import semmle.python.security.TaintTracking
import semmle.python.web.Http
import semmle.python.web.flask.General
private Object theFlaskRequestObject() {
result = theFlaskModule().attr("request")
}
private Value theFlaskRequestObject() { result = Value::named("flask.request") }
/** Holds if `attr` is an access of attribute `name` of the flask request object */
private predicate flask_request_attr(AttrNode attr, string name) {
attr.isLoad() and
attr.getObject(name).refersTo(theFlaskRequestObject())
attr.getObject(name).pointsTo(theFlaskRequestObject())
}
/** Source of external data from a flask request */
class FlaskRequestData extends HttpRequestTaintSource {
FlaskRequestData() {
not this instanceof FlaskRequestArgs and
exists(string name |
flask_request_attr(this, name) |
name = "path" or name = "full_path" or
name = "base_url" or name = "url"
exists(string name | flask_request_attr(this, name) |
name = "path" or
name = "full_path" or
name = "base_url" or
name = "url"
)
}
override predicate isSourceOf(TaintKind kind) {
kind instanceof ExternalStringKind
}
override string toString() {
result = "flask.request"
}
override predicate isSourceOf(TaintKind kind) { kind instanceof ExternalStringKind }
override string toString() { result = "flask.request" }
}
/** Source of dictionary whose values are externally controlled */
class FlaskRequestArgs extends HttpRequestTaintSource {
FlaskRequestArgs() {
exists(string attr |
flask_request_attr(this, attr) |
attr = "args" or attr = "form" or
attr = "values" or attr = "files" or
attr = "headers" or attr = "json"
exists(string attr | flask_request_attr(this, attr) |
attr = "args" or
attr = "form" or
attr = "values" or
attr = "files" or
attr = "headers" or
attr = "json"
)
}
override predicate isSourceOf(TaintKind kind) {
kind instanceof ExternalStringDictKind
}
override string toString() {
result = "flask.request.args"
}
override predicate isSourceOf(TaintKind kind) { kind instanceof ExternalStringDictKind }
override string toString() { result = "flask.request.args" }
}
/** Source of dictionary whose values are externally controlled */
class FlaskRequestJson extends TaintSource {
FlaskRequestJson() { flask_request_attr(this, "json") }
FlaskRequestJson() {
flask_request_attr(this, "json")
}
override predicate isSourceOf(TaintKind kind) {
kind instanceof ExternalJsonKind
}
override string toString() {
result = "flask.request.json"
}
override predicate isSourceOf(TaintKind kind) { kind instanceof ExternalJsonKind }
override string toString() { result = "flask.request.json" }
}

View File

@@ -1,15 +1,13 @@
import python
import semmle.python.security.TaintTracking
import semmle.python.security.strings.Basic
import semmle.python.web.flask.General
/** A flask response, which is vulnerable to any sort of
* http response malice. */
/**
* A flask response, which is vulnerable to any sort of
* http response malice.
*/
class FlaskRoutedResponse extends HttpResponseTaintSink {
FlaskRoutedResponse() {
exists(PyFunctionObject response |
flask_routing(_, response.getFunction()) and
@@ -17,32 +15,41 @@ class FlaskRoutedResponse extends HttpResponseTaintSink {
)
}
override predicate sinks(TaintKind kind) {
kind instanceof StringKind
}
override string toString() {
result = "flask.routed.response"
}
override predicate sinks(TaintKind kind) { kind instanceof StringKind }
override string toString() { result = "flask.routed.response" }
}
class FlaskResponseArgument extends HttpResponseTaintSink {
FlaskResponseArgument() {
exists(CallNode call |
call.getFunction().refersTo(theFlaskReponseClass()) and
(
call.getFunction().pointsTo(theFlaskReponseClass())
or
call.getFunction().pointsTo(Value::named("flask.make_response"))
) and
call.getArg(0) = this
)
}
override predicate sinks(TaintKind kind) {
kind instanceof StringKind
}
override predicate sinks(TaintKind kind) { kind instanceof StringKind }
override string toString() {
result = "flask.response.argument"
}
override string toString() { result = "flask.response.argument" }
}
}
class FlaskResponseTaintKind extends TaintKind {
FlaskResponseTaintKind() { this = "flask.Response" }
}
class FlaskResponseConfiguration extends TaintTracking::Configuration {
FlaskResponseConfiguration() { this = "Flask response configuration" }
override predicate isSource(DataFlow::Node node, TaintKind kind) {
kind instanceof FlaskResponseTaintKind and
(
node.asCfgNode().(CallNode).getFunction().pointsTo(theFlaskReponseClass())
or
node.asCfgNode().(CallNode).getFunction().pointsTo(Value::named("flask.make_response"))
)
}
}

View File

@@ -0,0 +1,6 @@
| / | Function hello |
| /dangerous | Function dangerous |
| /dangerous-with-cfg-split | Function dangerous2 |
| /safe | Function safe |
| /the/ | Function get |
| /unsafe | Function unsafe |

View File

@@ -0,0 +1,9 @@
import python
import semmle.python.web.flask.General
from ControlFlowNode regex, Function func
where flask_routing(regex, func)
select regex.getNode().(StrConst).getText(), func.toString()

View File

@@ -0,0 +1,8 @@
| test.py:8 | Str | externally controlled string |
| test.py:29 | Attribute() | externally controlled string |
| test.py:35 | Subscript | externally controlled string |
| test.py:36 | None | externally controlled string |
| test.py:41 | BinaryExpr | externally controlled string |
| test.py:41 | make_response() | externally controlled string |
| test.py:46 | BinaryExpr | externally controlled string |
| test.py:46 | make_response() | externally controlled string |

View File

@@ -0,0 +1,10 @@
import python
import semmle.python.web.HttpRequest
import semmle.python.web.HttpResponse
import semmle.python.security.strings.Untrusted
from TaintSink sink, TaintKind kind
where sink.sinks(kind)
select sink.getLocation().toString(), sink.(ControlFlowNode).getNode().toString(), kind

View File

@@ -0,0 +1,6 @@
| test.py:22 | Attribute() | flask/MyView.as.view |
| test.py:29 | Attribute | {externally controlled string} |
| test.py:33 | Attribute | {externally controlled string} |
| test.py:35 | Attribute | {externally controlled string} |
| test.py:40 | Attribute | {externally controlled string} |
| test.py:45 | Attribute | {externally controlled string} |

View File

@@ -0,0 +1,11 @@
import python
import semmle.python.web.HttpRequest
import semmle.python.web.HttpResponse
import semmle.python.security.strings.Untrusted
from TaintSource src, TaintKind kind
where src.isSourceOf(kind)
select src.getLocation().toString(), src.(ControlFlowNode).getNode().toString(), kind

View File

@@ -0,0 +1,17 @@
| test.py:22 | Attribute() | flask/MyView.as.view |
| test.py:25 | the_view | flask/MyView.as.view |
| test.py:29 | Attribute | {externally controlled string} |
| test.py:29 | Attribute() | externally controlled string |
| test.py:33 | Attribute | {externally controlled string} |
| test.py:33 | Subscript | externally controlled string |
| test.py:35 | Attribute | {externally controlled string} |
| test.py:35 | Subscript | externally controlled string |
| test.py:40 | Attribute | {externally controlled string} |
| test.py:40 | Attribute() | externally controlled string |
| test.py:41 | BinaryExpr | externally controlled string |
| test.py:41 | first_name | externally controlled string |
| test.py:41 | make_response() | flask.Response |
| test.py:45 | Attribute | {externally controlled string} |
| test.py:45 | Attribute() | externally controlled string |
| test.py:46 | first_name | externally controlled string |
| test.py:46 | make_response() | flask.Response |

View File

@@ -0,0 +1,12 @@
import python
import semmle.python.web.HttpRequest
import semmle.python.web.HttpResponse
import semmle.python.security.strings.Untrusted
from TaintedNode node
where node.getLocation().getFile().getName().matches("%test.py")
select node.getLocation().toString(), node.getAstNode().toString(), node.getTaintKind()

View File

@@ -0,0 +1,2 @@
semmle-extractor-options: --max-import-depth=3 --lang=3 -p ../../../query-tests/Security/lib/
optimize: true

View File

@@ -0,0 +1,46 @@
import flask
from flask import Flask, request, make_response
app = Flask(__name__)
@app.route("/")
def hello():
return "Hello World!"
from flask.views import MethodView
class MyView(MethodView):
def get(self, user_id):
if user_id is None:
# return a list of users
pass
else:
# expose a single user
pass
the_view = MyView.as_view('my_view')
app.add_url_rule('/the/', defaults={'user_id': None},
view_func=the_view, methods=['GET',])
@app.route("/dangerous")
def dangerous():
return request.args.get('payload')
@app.route("/dangerous-with-cfg-split")
def dangerous2():
x = request.form['param0']
if request.method == "POST":
return request.form['param1']
return None
@app.route('/unsafe')
def unsafe():
first_name = request.args.get('name', '')
return make_response("Your name is " + first_name)
@app.route('/safe')
def safe():
first_name = request.args.get('name', '')
return make_response("Your name is " + escape(first_name))

View File

@@ -1,13 +1,9 @@
edges
| ../lib/flask/__init__.py:14:19:14:20 | externally controlled string | ../lib/flask/__init__.py:16:25:16:26 | externally controlled string |
| ../lib/flask/__init__.py:14:19:14:20 | externally controlled string | ../lib/flask/__init__.py:16:25:16:26 | externally controlled string |
| reflected_xss.py:7:18:7:29 | dict of externally controlled string | reflected_xss.py:7:18:7:45 | externally controlled string |
| reflected_xss.py:7:18:7:29 | dict of externally controlled string | reflected_xss.py:7:18:7:45 | externally controlled string |
| reflected_xss.py:7:18:7:45 | externally controlled string | reflected_xss.py:8:44:8:53 | externally controlled string |
| reflected_xss.py:7:18:7:45 | externally controlled string | reflected_xss.py:8:44:8:53 | externally controlled string |
| reflected_xss.py:8:26:8:53 | externally controlled string | ../lib/flask/__init__.py:14:19:14:20 | externally controlled string |
| reflected_xss.py:8:26:8:53 | externally controlled string | ../lib/flask/__init__.py:14:19:14:20 | externally controlled string |
| reflected_xss.py:8:44:8:53 | externally controlled string | reflected_xss.py:8:26:8:53 | externally controlled string |
| reflected_xss.py:8:44:8:53 | externally controlled string | reflected_xss.py:8:26:8:53 | externally controlled string |
#select
| ../lib/flask/__init__.py:16:25:16:26 | rv | reflected_xss.py:7:18:7:29 | dict of externally controlled string | ../lib/flask/__init__.py:16:25:16:26 | externally controlled string | Cross-site scripting vulnerability due to $@. | reflected_xss.py:7:18:7:29 | Attribute | user-provided value |
| reflected_xss.py:8:26:8:53 | BinaryExpr | reflected_xss.py:7:18:7:29 | dict of externally controlled string | reflected_xss.py:8:26:8:53 | externally controlled string | Cross-site scripting vulnerability due to $@. | reflected_xss.py:7:18:7:29 | Attribute | user-provided value |

View File

@@ -11,8 +11,3 @@ def unsafe():
def safe():
first_name = request.args.get('name', '')
return make_response("Your name is " + escape(first_name))
urlpatterns = [
url(r'^r1$', response_unsafe, name='response-unsafe'),
url(r'^r2$', response_safe, name='response-safe')
]

View File

@@ -1,5 +1,6 @@
edges
| password_in_cookie.py:7:16:7:43 | a password | password_in_cookie.py:9:33:9:40 | a password |
| password_in_cookie.py:14:16:14:43 | a password | password_in_cookie.py:16:33:16:40 | a password |
| test.py:7:16:7:29 | a password | test.py:8:35:8:42 | a password |
| test.py:7:16:7:29 | a password | test.py:8:35:8:42 | a password |
| test.py:20:12:20:21 | a certificate or key | test.py:22:20:22:23 | a certificate or key |

View File

@@ -1,9 +1,12 @@
edges
| password_in_cookie.py:7:16:7:43 | a password | password_in_cookie.py:9:33:9:40 | a password |
| password_in_cookie.py:7:16:7:43 | a password | password_in_cookie.py:9:33:9:40 | a password |
| password_in_cookie.py:14:16:14:43 | a password | password_in_cookie.py:16:33:16:40 | a password |
| password_in_cookie.py:14:16:14:43 | a password | password_in_cookie.py:16:33:16:40 | a password |
| test.py:7:16:7:29 | a password | test.py:8:35:8:42 | a password |
| test.py:20:12:20:21 | a certificate or key | test.py:22:20:22:23 | a certificate or key |
| test.py:20:12:20:21 | a certificate or key | test.py:22:20:22:23 | a certificate or key |
#select
| password_in_cookie.py:9:33:9:40 | password | password_in_cookie.py:7:16:7:43 | a password | password_in_cookie.py:9:33:9:40 | a password | Sensitive data from $@ is stored here. | password_in_cookie.py:7:16:7:43 | Attribute() | a request parameter containing a password |
| password_in_cookie.py:16:33:16:40 | password | password_in_cookie.py:14:16:14:43 | a password | password_in_cookie.py:16:33:16:40 | a password | Sensitive data from $@ is stored here. | password_in_cookie.py:14:16:14:43 | Attribute() | a request parameter containing a password |
| test.py:22:20:22:23 | cert | test.py:20:12:20:21 | a certificate or key | test.py:22:20:22:23 | a certificate or key | Sensitive data from $@ is stored here. | test.py:20:12:20:21 | get_cert() | a call returning a certificate or key |

View File

@@ -1,4 +1,4 @@
from flask import Flask, make_response, request
from flask import Flask, make_response, request, Response
app = Flask("Leak password")
@@ -8,3 +8,10 @@ def index():
resp = make_response(render_template(...))
resp.set_cookie("password", password)
return resp
@app.route('/')
def index2():
password = request.args.get("password")
resp = Response(...)
resp.set_cookie("password", password)
return resp

View File

@@ -1,9 +1,17 @@
class Flask(object):
def run(self, *args, **kwargs): pass
from .globals import request
from .globals import current_app
class Flask(object):
# Only some methods mocked, signature copied from
# https://flask.palletsprojects.com/en/1.1.x/api/#flask.Flask
def run(host=None, port=None, debug=None, load_dotenv=True, **options):
pass
def make_response(rv):
pass
def add_url_rule(rule, endpoint=None, view_func=None, provide_automatic_options=None, **options):
pass
class Response(object):
pass
@@ -12,12 +20,11 @@ def redirect(location, code=302, Response=None):
pass
def make_response(rv):
if isinstance(rv, str):
return Response(rv)
elif isinstance(rv, Response):
return rv
else:
pass
if not args:
return current_app.response_class()
if len(args) == 1:
args = args[0]
return current_app.make_response(args)
def escape(txt):
return Markup.escape(txt)

View File

@@ -3,3 +3,4 @@ class LocalProxy(object):
pass
request = LocalProxy()
current_app = LocalProxy()

View File

@@ -1,11 +1,6 @@
class View(object):
pass
class MethodView(object):
pass