From 8d525e5295fc0991c1fd12149c8a38edf7e7dbac Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 4 Feb 2019 11:51:55 +0000 Subject: [PATCH 1/4] Python: Add support for bottle framework routing and requests. --- .../ql/src/semmle/python/web/HttpRequest.qll | 1 + .../src/semmle/python/web/bottle/General.qll | 79 ++++++++++++ .../src/semmle/python/web/bottle/Request.qll | 115 ++++++++++++++++++ .../library-tests/web/bottle/Routing.expected | 3 + .../test/library-tests/web/bottle/Routing.ql | 7 ++ .../library-tests/web/bottle/Sources.expected | 7 ++ .../test/library-tests/web/bottle/Sources.ql | 10 ++ .../ql/test/library-tests/web/bottle/options | 2 + .../ql/test/library-tests/web/bottle/test.py | 19 +++ .../test/query-tests/Security/lib/bottle.py | 66 ++++++++++ 10 files changed, 309 insertions(+) create mode 100644 python/ql/src/semmle/python/web/bottle/General.qll create mode 100644 python/ql/src/semmle/python/web/bottle/Request.qll create mode 100644 python/ql/test/library-tests/web/bottle/Routing.expected create mode 100644 python/ql/test/library-tests/web/bottle/Routing.ql create mode 100644 python/ql/test/library-tests/web/bottle/Sources.expected create mode 100644 python/ql/test/library-tests/web/bottle/Sources.ql create mode 100644 python/ql/test/library-tests/web/bottle/options create mode 100644 python/ql/test/library-tests/web/bottle/test.py create mode 100644 python/ql/test/query-tests/Security/lib/bottle.py diff --git a/python/ql/src/semmle/python/web/HttpRequest.qll b/python/ql/src/semmle/python/web/HttpRequest.qll index 1566ac645dcd..c2807f3c4a3b 100644 --- a/python/ql/src/semmle/python/web/HttpRequest.qll +++ b/python/ql/src/semmle/python/web/HttpRequest.qll @@ -3,3 +3,4 @@ import semmle.python.web.flask.Request import semmle.python.web.tornado.Request import semmle.python.web.pyramid.Request import semmle.python.web.twisted.Request +import semmle.python.web.bottle.Request diff --git a/python/ql/src/semmle/python/web/bottle/General.qll b/python/ql/src/semmle/python/web/bottle/General.qll new file mode 100644 index 000000000000..6253029fbbd0 --- /dev/null +++ b/python/ql/src/semmle/python/web/bottle/General.qll @@ -0,0 +1,79 @@ +import python +import semmle.python.web.Http +import semmle.python.types.Extensions + +/** The flask module */ +ModuleObject theBottleModule() { + result = ModuleObject::named("bottle") +} + +/** The flask app class */ +ClassObject theBottleClass() { + result = ModuleObject::named("bottle").getAttribute("Bottle") +} + +/** Holds if `route` is routed to `func` + * by decorating `func` with `app.route(route)` or `route(route)` + */ +predicate bottle_route(CallNode route_call, ControlFlowNode route, Function func) { + exists(CallNode decorator_call, string name | + route_call.getFunction().(AttrNode).getObject(name).refersTo(_, theBottleClass(), _) or + route_call.getFunction().refersTo(theBottleModule().getAttribute(name)) + | + (name = "route" or name = httpVerbLower()) and + decorator_call.getFunction() = route_call and + route_call.getArg(0) = route and + decorator_call.getArg(0).getNode().(FunctionExpr).getInnerScope() = func + ) +} + +class BottleRoute extends ControlFlowNode { + + BottleRoute() { + bottle_route(this, _, _) + } + + string getUrl() { + exists(StrConst url | + bottle_route(this, url.getAFlowNode(), _) and + result = url.getText() + ) + } + + Function getFunction() { + bottle_route(this, _, result) + } + + Parameter getNamedArgument() { + exists(string name, Function func | + func = this.getFunction() and + func.getArgByName(name) = result and + this.getUrl().matches("%<" + name + ">%") + ) + } +} + + +/* bottle module route constants */ + +class BottleRoutePointToExtension extends CustomPointsToFact { + + string name; + + BottleRoutePointToExtension() { + exists(DefinitionNode defn | + defn.getScope().(Module).getName() = "bottle" and + this = defn.getValue() and + name = defn.(NameNode).getId() + | + name = "route" or + name = httpVerbLower() + ) + } + + override predicate pointsTo(Context context, Object value, ClassObject cls, ControlFlowNode origin) { + context.isImport() and + ModuleObject::named("bottle").getAttribute("Bottle").(ClassObject).attributeRefersTo(name, value, cls, origin) + } +} + diff --git a/python/ql/src/semmle/python/web/bottle/Request.qll b/python/ql/src/semmle/python/web/bottle/Request.qll new file mode 100644 index 000000000000..4eef542e0cc8 --- /dev/null +++ b/python/ql/src/semmle/python/web/bottle/Request.qll @@ -0,0 +1,115 @@ +import python + + +import semmle.python.security.TaintTracking +import semmle.python.security.strings.Untrusted +import semmle.python.web.Http +import semmle.python.web.bottle.General + +private Object theBottleRequestObject() { + result = theBottleModule().getAttribute("request") +} + +class BottleRequestKind extends TaintKind { + + BottleRequestKind() { + this = "bottle.request" + } + + override TaintKind getTaintOfAttribute(string name) { + result instanceof BottleFormsDict and + (name = "cookies" or name = "query" or name = "form") + or + result instanceof UntrustedStringKind and + (name = "query_string" or name = "url_args") + or + result.(DictKind).getValue() instanceof FileUpload and + name = "files" + } + +} + +private class RequestSource extends TaintSource { + + RequestSource() { + this.(ControlFlowNode).refersTo(theBottleRequestObject()) + } + + override predicate isSourceOf(TaintKind kind) { + kind instanceof BottleRequestKind + } + +} + + +class BottleFormsDict extends TaintKind { + + BottleFormsDict() { + this = "bottle.FormsDict" + } + + override TaintKind getTaintForFlowStep(ControlFlowNode fromnode, ControlFlowNode tonode) { + /* Cannot use `getTaintOfAttribute()` as it doesn't bind name */ + exists(string name | + tonode = fromnode.(AttrNode).getObject(name) and + result instanceof UntrustedStringKind + | + name != "get" and name != "getunicode" and name != "getall" + ) + } + + override TaintKind getTaintOfMethodResult(string name) { + (name = "get" or name = "getunicode") and + result instanceof UntrustedStringKind + or + name = "getall" and result.(SequenceKind).getItem() instanceof UntrustedStringKind + } +} + +class FileUpload extends TaintKind { + + FileUpload() { + this = "bottle.FileUpload" + } + + override TaintKind getTaintOfAttribute(string name) { + name = "filename" and result instanceof UntrustedStringKind + or + name = "raw_filename" and result instanceof UntrustedStringKind + or + name = "file" and result instanceof UntrustedFile + } + +} + +class UntrustedFile extends TaintKind { + + UntrustedFile() { this = "Untrusted file" } + +} + +// +// TO DO.. File uploads -- Should check about file uploads for other frameworks as well. +// Move UntrustedFile to shared location +// + + +/** Parameter to a bottle request handler function */ +class BottleRequestParameter extends TaintSource { + + BottleRequestParameter() { + exists(BottleRoute route | + route.getNamedArgument() = this.(ControlFlowNode).getNode() + ) + } + + override predicate isSourceOf(TaintKind kind) { + kind instanceof UntrustedStringKind + } + + override string toString() { + result = "flask.request.args" + } + +} + diff --git a/python/ql/test/library-tests/web/bottle/Routing.expected b/python/ql/test/library-tests/web/bottle/Routing.expected new file mode 100644 index 000000000000..8932ff0100f9 --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Routing.expected @@ -0,0 +1,3 @@ +| /bye/ | test.py:12:1:12:25 | Function bye | +| /hello/ | test.py:8:1:8:27 | Function hello | +| /other | test.py:17:1:17:12 | Function other | diff --git a/python/ql/test/library-tests/web/bottle/Routing.ql b/python/ql/test/library-tests/web/bottle/Routing.ql new file mode 100644 index 000000000000..3a10c78ddcba --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Routing.ql @@ -0,0 +1,7 @@ +import python + +import semmle.python.web.bottle.General + +from BottleRoute route + +select route.getUrl(), route.getFunction() diff --git a/python/ql/test/library-tests/web/bottle/Sources.expected b/python/ql/test/library-tests/web/bottle/Sources.expected new file mode 100644 index 000000000000..c197b4954ee5 --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Sources.expected @@ -0,0 +1,7 @@ +| ../../../query-tests/Security/lib/bottle.py:64 | LocalRequest() | bottle.request | +| ../../../query-tests/Security/lib/bottle.py:64 | request | bottle.request | +| test.py:3 | ImportMember | bottle.request | +| test.py:3 | request | bottle.request | +| test.py:8 | name | externally controlled string | +| test.py:12 | name | externally controlled string | +| test.py:18 | request | bottle.request | diff --git a/python/ql/test/library-tests/web/bottle/Sources.ql b/python/ql/test/library-tests/web/bottle/Sources.ql new file mode 100644 index 000000000000..c1b9cc82e197 --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Sources.ql @@ -0,0 +1,10 @@ + +import python + +import semmle.python.web.HttpRequest +import semmle.python.security.strings.Untrusted + + +from TaintSource src, TaintKind kind +where src.isSourceOf(kind) and not kind.matches("tornado%") +select src.getLocation().toString(), src.(ControlFlowNode).getNode().toString(), kind diff --git a/python/ql/test/library-tests/web/bottle/options b/python/ql/test/library-tests/web/bottle/options new file mode 100644 index 000000000000..3eb8fa37213e --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/options @@ -0,0 +1,2 @@ +semmle-extractor-options: --max-import-depth=3 --lang=3 -p ../../../query-tests/Security/lib/ +optimize: true diff --git a/python/ql/test/library-tests/web/bottle/test.py b/python/ql/test/library-tests/web/bottle/test.py new file mode 100644 index 000000000000..251a2bd8d27f --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/test.py @@ -0,0 +1,19 @@ + + +from bottle import Bottle, route, request + +app = Bottle() + +@app.route('/hello/') +def hello(name = "World!"): + return "Hello " + name + +@route('/bye/') +def bye(name = "World!"): + return "Bye " + name + + +@route('/other') +def other(): + name = request.cookies.username + return "User name is " + name diff --git a/python/ql/test/query-tests/Security/lib/bottle.py b/python/ql/test/query-tests/Security/lib/bottle.py new file mode 100644 index 000000000000..015dc15af81e --- /dev/null +++ b/python/ql/test/query-tests/Security/lib/bottle.py @@ -0,0 +1,66 @@ + +class Bottle(object): + + def route(self, path=None, method='GET', **options): + pass + + def get(self, path=None, method='GET', **options): + """ Equals :meth:`route`. """ + return self.route(path, method, **options) + + def post(self, path=None, method='POST', **options): + """ Equals :meth:`route` with a ``POST`` method parameter. """ + return self.route(path, method, **options) + + def put(self, path=None, method='PUT', **options): + """ Equals :meth:`route` with a ``PUT`` method parameter. """ + return self.route(path, method, **options) + + def delete(self, path=None, method='DELETE', **options): + """ Equals :meth:`route` with a ``DELETE`` method parameter. """ + return self.route(path, method, **options) + + def error(self, code=500): + """ Decorator: Register an output handler for a HTTP error code""" + def wrapper(handler): + self.error_handler[int(code)] = handler + return handler + return wrapper + +#Use same wrapper logic as the original `bottle` code. + +def make_default_app_wrapper(name): + """ Return a callable that relays calls to the current default app. """ + + @functools.wraps(getattr(Bottle, name)) + def wrapper(*a, **ka): + return getattr(app(), name)(*a, **ka) + + return wrapper + +route = make_default_app_wrapper('route') +get = make_default_app_wrapper('get') +post = make_default_app_wrapper('post') +put = make_default_app_wrapper('put') +delete = make_default_app_wrapper('delete') +patch = make_default_app_wrapper('patch') +error = make_default_app_wrapper('error') +mount = make_default_app_wrapper('mount') +hook = make_default_app_wrapper('hook') +install = make_default_app_wrapper('install') +uninstall = make_default_app_wrapper('uninstall') +url = make_default_app_wrapper('get_url') + +class LocalProxy(object): + pass + +class LocalRequest(LocalProxy): + pass + +class LocalResponse(LocalProxy): + pass + + +request = LocalRequest() +response = LocalResponse() + From d514fc543d20f67f473ae186490e38a490e174f7 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 4 Feb 2019 14:35:30 +0000 Subject: [PATCH 2/4] Python: Add responses to bottle framework support. --- .../ql/src/semmle/python/web/HttpResponse.qll | 1 + .../src/semmle/python/web/bottle/Request.qll | 6 +- .../src/semmle/python/web/bottle/Response.qll | 58 +++++++++++++++++++ .../library-tests/web/bottle/Sinks.expected | 3 + .../ql/test/library-tests/web/bottle/Sinks.ql | 10 ++++ .../library-tests/web/bottle/Taint.expected | 15 +++++ .../ql/test/library-tests/web/bottle/Taint.ql | 13 +++++ 7 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 python/ql/src/semmle/python/web/bottle/Response.qll create mode 100644 python/ql/test/library-tests/web/bottle/Sinks.expected create mode 100644 python/ql/test/library-tests/web/bottle/Sinks.ql create mode 100644 python/ql/test/library-tests/web/bottle/Taint.expected create mode 100644 python/ql/test/library-tests/web/bottle/Taint.ql diff --git a/python/ql/src/semmle/python/web/HttpResponse.qll b/python/ql/src/semmle/python/web/HttpResponse.qll index f38836d768b4..57fc8806bf0b 100644 --- a/python/ql/src/semmle/python/web/HttpResponse.qll +++ b/python/ql/src/semmle/python/web/HttpResponse.qll @@ -3,3 +3,4 @@ import semmle.python.web.flask.Response import semmle.python.web.pyramid.Response import semmle.python.web.tornado.Response import semmle.python.web.twisted.Response +import semmle.python.web.bottle.Response diff --git a/python/ql/src/semmle/python/web/bottle/Request.qll b/python/ql/src/semmle/python/web/bottle/Request.qll index 4eef542e0cc8..4039a1aa0bd6 100644 --- a/python/ql/src/semmle/python/web/bottle/Request.qll +++ b/python/ql/src/semmle/python/web/bottle/Request.qll @@ -49,9 +49,9 @@ class BottleFormsDict extends TaintKind { } override TaintKind getTaintForFlowStep(ControlFlowNode fromnode, ControlFlowNode tonode) { - /* Cannot use `getTaintOfAttribute()` as it doesn't bind name */ + /* Cannot use `getTaintOfAttribute(name)` as it wouldn't bind `name` */ exists(string name | - tonode = fromnode.(AttrNode).getObject(name) and + fromnode = tonode.(AttrNode).getObject(name) and result instanceof UntrustedStringKind | name != "get" and name != "getunicode" and name != "getall" @@ -108,7 +108,7 @@ class BottleRequestParameter extends TaintSource { } override string toString() { - result = "flask.request.args" + result = "bottle handler function argument" } } diff --git a/python/ql/src/semmle/python/web/bottle/Response.qll b/python/ql/src/semmle/python/web/bottle/Response.qll new file mode 100644 index 000000000000..09527e84198b --- /dev/null +++ b/python/ql/src/semmle/python/web/bottle/Response.qll @@ -0,0 +1,58 @@ +import python + +import semmle.python.security.TaintTracking +import semmle.python.security.strings.Untrusted +import semmle.python.web.Http +import semmle.python.web.bottle.General + + +/** A django.http.response.Response object + * This isn't really a "taint", but we use the value tracking machinery to + * track the flow of response objects. + */ +class BottleResponse extends TaintKind { + + BottleResponse() { + this = "bottle.response" + } + +} + +private Object theBottleResponseObject() { + result = theBottleModule().getAttribute("request") +} + +class BottleResponseBodyAssignment extends TaintSink { + + BottleResponseBodyAssignment() { + exists(DefinitionNode lhs | + lhs.getValue() = this and + lhs.(AttrNode).getObject("body").refersTo(theBottleResponseObject()) + ) + } + + override predicate sinks(TaintKind kind) { + kind instanceof StringKind + } + +} + +class BottleHandlerFunctionResult extends TaintSink { + + BottleHandlerFunctionResult() { + exists(BottleRoute route, Return ret | + ret.getScope() = route.getFunction() and + ret.getValue().getAFlowNode() = this + ) + } + + override predicate sinks(TaintKind kind) { + kind instanceof UntrustedStringKind + } + + override string toString() { + result = "bottle handler function result" + } + +} + diff --git a/python/ql/test/library-tests/web/bottle/Sinks.expected b/python/ql/test/library-tests/web/bottle/Sinks.expected new file mode 100644 index 000000000000..c5f929c07a6a --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Sinks.expected @@ -0,0 +1,3 @@ +| test.py:9 | BinaryExpr | externally controlled string | +| test.py:13 | BinaryExpr | externally controlled string | +| test.py:19 | BinaryExpr | externally controlled string | diff --git a/python/ql/test/library-tests/web/bottle/Sinks.ql b/python/ql/test/library-tests/web/bottle/Sinks.ql new file mode 100644 index 000000000000..34aa1cfc429c --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Sinks.ql @@ -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 diff --git a/python/ql/test/library-tests/web/bottle/Taint.expected b/python/ql/test/library-tests/web/bottle/Taint.expected new file mode 100644 index 000000000000..3d50ff99fb9c --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Taint.expected @@ -0,0 +1,15 @@ +| ../../../query-tests/Security/lib/bottle.py:64 | LocalRequest() | bottle.request | +| ../../../query-tests/Security/lib/bottle.py:64 | request | bottle.request | +| test.py:3 | ImportMember | bottle.request | +| test.py:3 | request | bottle.request | +| test.py:8 | name | externally controlled string | +| test.py:9 | BinaryExpr | externally controlled string | +| test.py:9 | name | externally controlled string | +| test.py:12 | name | externally controlled string | +| test.py:13 | BinaryExpr | externally controlled string | +| test.py:13 | name | externally controlled string | +| test.py:18 | Attribute | bottle.FormsDict | +| test.py:18 | Attribute | externally controlled string | +| test.py:18 | request | bottle.request | +| test.py:19 | BinaryExpr | externally controlled string | +| test.py:19 | name | externally controlled string | diff --git a/python/ql/test/library-tests/web/bottle/Taint.ql b/python/ql/test/library-tests/web/bottle/Taint.ql new file mode 100644 index 000000000000..6699c9bd9dae --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Taint.ql @@ -0,0 +1,13 @@ + +import python + + +import semmle.python.web.HttpRequest +import semmle.python.web.HttpResponse +import semmle.python.security.strings.Untrusted + + +from TaintedNode node + +select node.getLocation().toString(), node.getNode().getNode().toString(), node.getTaintKind() + From aab0a243dcfd4454931b44633a27d8b4f3333bd1 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 4 Feb 2019 15:04:25 +0000 Subject: [PATCH 3/4] Python: Add redirects to bottle framework support. --- change-notes/1.20/analysis-python.md | 2 ++ .../ql/src/semmle/python/web/HttpRedirect.qll | 1 + .../src/semmle/python/web/bottle/Redirect.qll | 35 +++++++++++++++++++ .../library-tests/web/bottle/Routing.expected | 3 ++ .../library-tests/web/bottle/Sources.expected | 2 ++ .../library-tests/web/bottle/Taint.expected | 6 ++++ .../ql/test/library-tests/web/bottle/test.py | 15 +++++++- .../test/query-tests/Security/lib/bottle.py | 4 +++ 8 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 python/ql/src/semmle/python/web/bottle/Redirect.qll diff --git a/change-notes/1.20/analysis-python.md b/change-notes/1.20/analysis-python.md index b620e2350913..19a6f2a14ab5 100644 --- a/change-notes/1.20/analysis-python.md +++ b/change-notes/1.20/analysis-python.md @@ -35,3 +35,5 @@ Removes false positives seen when using Python 3.6, but not when using earlier v ## Changes to QL libraries * Added support for the `dill` pickle library. + * Added support for the bottle web framework. + diff --git a/python/ql/src/semmle/python/web/HttpRedirect.qll b/python/ql/src/semmle/python/web/HttpRedirect.qll index f3df7cac80d2..e77f08411956 100644 --- a/python/ql/src/semmle/python/web/HttpRedirect.qll +++ b/python/ql/src/semmle/python/web/HttpRedirect.qll @@ -6,3 +6,4 @@ import semmle.python.web.django.Redirect import semmle.python.web.flask.Redirect import semmle.python.web.tornado.Redirect import semmle.python.web.pyramid.Redirect +import semmle.python.web.bottle.Redirect diff --git a/python/ql/src/semmle/python/web/bottle/Redirect.qll b/python/ql/src/semmle/python/web/bottle/Redirect.qll new file mode 100644 index 000000000000..f834612d5c2a --- /dev/null +++ b/python/ql/src/semmle/python/web/bottle/Redirect.qll @@ -0,0 +1,35 @@ +/** Provides class representing the `bottle.redirect` function. + * This module is intended to be imported into a taint-tracking query + * to extend `TaintSink`. + */ +import python + +import semmle.python.security.TaintTracking +import semmle.python.security.strings.Basic +import semmle.python.web.bottle.General + +FunctionObject bottle_redirect() { + result = theBottleModule().getAttribute("redirect") +} + +/** + * Represents an argument to the `bottle.redirect` function. + */ +class BottleRedirect extends TaintSink { + + override string toString() { + result = "bottle.redirect" + } + + BottleRedirect() { + exists(CallNode call | + bottle_redirect().getACall() = call and + this = call.getAnArg() + ) + } + + override predicate sinks(TaintKind kind) { + kind instanceof StringKind + } + +} diff --git a/python/ql/test/library-tests/web/bottle/Routing.expected b/python/ql/test/library-tests/web/bottle/Routing.expected index 8932ff0100f9..3ae308ae5448 100644 --- a/python/ql/test/library-tests/web/bottle/Routing.expected +++ b/python/ql/test/library-tests/web/bottle/Routing.expected @@ -1,3 +1,6 @@ +| /args | test.py:31:1:31:14 | Function unsafe2 | | /bye/ | test.py:12:1:12:25 | Function bye | | /hello/ | test.py:8:1:8:27 | Function hello | | /other | test.py:17:1:17:12 | Function other | +| /wrong/ | test.py:27:1:27:31 | Function unsafe | +| /wrong/url | test.py:23:1:23:11 | Function safe | diff --git a/python/ql/test/library-tests/web/bottle/Sources.expected b/python/ql/test/library-tests/web/bottle/Sources.expected index c197b4954ee5..f2886bafda47 100644 --- a/python/ql/test/library-tests/web/bottle/Sources.expected +++ b/python/ql/test/library-tests/web/bottle/Sources.expected @@ -5,3 +5,5 @@ | test.py:8 | name | externally controlled string | | test.py:12 | name | externally controlled string | | test.py:18 | request | bottle.request | +| test.py:27 | where | externally controlled string | +| test.py:32 | request | bottle.request | diff --git a/python/ql/test/library-tests/web/bottle/Taint.expected b/python/ql/test/library-tests/web/bottle/Taint.expected index 3d50ff99fb9c..aa10154cb6ef 100644 --- a/python/ql/test/library-tests/web/bottle/Taint.expected +++ b/python/ql/test/library-tests/web/bottle/Taint.expected @@ -1,5 +1,6 @@ | ../../../query-tests/Security/lib/bottle.py:64 | LocalRequest() | bottle.request | | ../../../query-tests/Security/lib/bottle.py:64 | request | bottle.request | +| ../../../query-tests/Security/lib/bottle.py:68 | url | externally controlled string | | test.py:3 | ImportMember | bottle.request | | test.py:3 | request | bottle.request | | test.py:8 | name | externally controlled string | @@ -13,3 +14,8 @@ | test.py:18 | request | bottle.request | | test.py:19 | BinaryExpr | externally controlled string | | test.py:19 | name | externally controlled string | +| test.py:27 | where | externally controlled string | +| test.py:28 | where | externally controlled string | +| test.py:32 | Attribute | bottle.FormsDict | +| test.py:32 | Attribute | externally controlled string | +| test.py:32 | request | bottle.request | diff --git a/python/ql/test/library-tests/web/bottle/test.py b/python/ql/test/library-tests/web/bottle/test.py index 251a2bd8d27f..ecdd99a23d01 100644 --- a/python/ql/test/library-tests/web/bottle/test.py +++ b/python/ql/test/library-tests/web/bottle/test.py @@ -1,6 +1,6 @@ -from bottle import Bottle, route, request +from bottle import Bottle, route, request, redirect app = Bottle() @@ -17,3 +17,16 @@ def bye(name = "World!"): def other(): name = request.cookies.username return "User name is " + name + + +@route('/wrong/url') +def safe(): + redirect("/right/url") + +@route('/wrong/') +def unsafe(where="/right/url"): + redirect(where) + +@route('/args') +def unsafe2(): + redirect(request.query.where, code) diff --git a/python/ql/test/query-tests/Security/lib/bottle.py b/python/ql/test/query-tests/Security/lib/bottle.py index 015dc15af81e..bd2d736c192e 100644 --- a/python/ql/test/query-tests/Security/lib/bottle.py +++ b/python/ql/test/query-tests/Security/lib/bottle.py @@ -64,3 +64,7 @@ class LocalResponse(LocalProxy): request = LocalRequest() response = LocalResponse() + +def redirect(url, code=None): + pass + From b644891e530ac3574cc2252ddb6c88d55d665f8b Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 11 Feb 2019 14:33:49 +0000 Subject: [PATCH 4/4] Python: Fix up some typos for bottle and add a few more tests. --- python/ql/src/semmle/python/web/bottle/General.qll | 4 ++-- python/ql/src/semmle/python/web/bottle/Response.qll | 6 +++--- python/ql/test/library-tests/web/bottle/Routing.expected | 1 + python/ql/test/library-tests/web/bottle/Sinks.expected | 1 + python/ql/test/library-tests/web/bottle/Sources.expected | 1 + python/ql/test/library-tests/web/bottle/Taint.expected | 4 ++++ python/ql/test/library-tests/web/bottle/test.py | 6 +++++- 7 files changed, 17 insertions(+), 6 deletions(-) diff --git a/python/ql/src/semmle/python/web/bottle/General.qll b/python/ql/src/semmle/python/web/bottle/General.qll index 6253029fbbd0..e91a26ec0d8a 100644 --- a/python/ql/src/semmle/python/web/bottle/General.qll +++ b/python/ql/src/semmle/python/web/bottle/General.qll @@ -2,12 +2,12 @@ import python import semmle.python.web.Http import semmle.python.types.Extensions -/** The flask module */ +/** The bottle module */ ModuleObject theBottleModule() { result = ModuleObject::named("bottle") } -/** The flask app class */ +/** The bottle.Bottle class */ ClassObject theBottleClass() { result = ModuleObject::named("bottle").getAttribute("Bottle") } diff --git a/python/ql/src/semmle/python/web/bottle/Response.qll b/python/ql/src/semmle/python/web/bottle/Response.qll index 09527e84198b..715f3448a7a6 100644 --- a/python/ql/src/semmle/python/web/bottle/Response.qll +++ b/python/ql/src/semmle/python/web/bottle/Response.qll @@ -6,7 +6,7 @@ import semmle.python.web.Http import semmle.python.web.bottle.General -/** A django.http.response.Response object +/** A bottle.Response object * This isn't really a "taint", but we use the value tracking machinery to * track the flow of response objects. */ @@ -19,7 +19,7 @@ class BottleResponse extends TaintKind { } private Object theBottleResponseObject() { - result = theBottleModule().getAttribute("request") + result = theBottleModule().getAttribute("response") } class BottleResponseBodyAssignment extends TaintSink { @@ -32,7 +32,7 @@ class BottleResponseBodyAssignment extends TaintSink { } override predicate sinks(TaintKind kind) { - kind instanceof StringKind + kind instanceof UntrustedStringKind } } diff --git a/python/ql/test/library-tests/web/bottle/Routing.expected b/python/ql/test/library-tests/web/bottle/Routing.expected index 3ae308ae5448..f4006d1caac3 100644 --- a/python/ql/test/library-tests/web/bottle/Routing.expected +++ b/python/ql/test/library-tests/web/bottle/Routing.expected @@ -4,3 +4,4 @@ | /other | test.py:17:1:17:12 | Function other | | /wrong/ | test.py:27:1:27:31 | Function unsafe | | /wrong/url | test.py:23:1:23:11 | Function safe | +| /xss | test.py:35:1:35:16 | Function maybe_xss | diff --git a/python/ql/test/library-tests/web/bottle/Sinks.expected b/python/ql/test/library-tests/web/bottle/Sinks.expected index c5f929c07a6a..64edd29140fe 100644 --- a/python/ql/test/library-tests/web/bottle/Sinks.expected +++ b/python/ql/test/library-tests/web/bottle/Sinks.expected @@ -1,3 +1,4 @@ | test.py:9 | BinaryExpr | externally controlled string | | test.py:13 | BinaryExpr | externally controlled string | | test.py:19 | BinaryExpr | externally controlled string | +| test.py:36 | BinaryExpr | externally controlled string | diff --git a/python/ql/test/library-tests/web/bottle/Sources.expected b/python/ql/test/library-tests/web/bottle/Sources.expected index f2886bafda47..ae780564c374 100644 --- a/python/ql/test/library-tests/web/bottle/Sources.expected +++ b/python/ql/test/library-tests/web/bottle/Sources.expected @@ -7,3 +7,4 @@ | test.py:18 | request | bottle.request | | test.py:27 | where | externally controlled string | | test.py:32 | request | bottle.request | +| test.py:36 | request | bottle.request | diff --git a/python/ql/test/library-tests/web/bottle/Taint.expected b/python/ql/test/library-tests/web/bottle/Taint.expected index aa10154cb6ef..ea97dab3473a 100644 --- a/python/ql/test/library-tests/web/bottle/Taint.expected +++ b/python/ql/test/library-tests/web/bottle/Taint.expected @@ -19,3 +19,7 @@ | test.py:32 | Attribute | bottle.FormsDict | | test.py:32 | Attribute | externally controlled string | | test.py:32 | request | bottle.request | +| test.py:36 | Attribute | bottle.FormsDict | +| test.py:36 | Attribute | externally controlled string | +| test.py:36 | BinaryExpr | externally controlled string | +| test.py:36 | request | bottle.request | diff --git a/python/ql/test/library-tests/web/bottle/test.py b/python/ql/test/library-tests/web/bottle/test.py index ecdd99a23d01..8975de72ea43 100644 --- a/python/ql/test/library-tests/web/bottle/test.py +++ b/python/ql/test/library-tests/web/bottle/test.py @@ -1,6 +1,6 @@ -from bottle import Bottle, route, request, redirect +from bottle import Bottle, route, request, redirect, response app = Bottle() @@ -30,3 +30,7 @@ def unsafe(where="/right/url"): @route('/args') def unsafe2(): redirect(request.query.where, code) + +@route('/xss') +def maybe_xss(): + response.body = "name is " + request.query.name