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/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/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/General.qll b/python/ql/src/semmle/python/web/bottle/General.qll new file mode 100644 index 000000000000..e91a26ec0d8a --- /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 bottle module */ +ModuleObject theBottleModule() { + result = ModuleObject::named("bottle") +} + +/** The bottle.Bottle 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/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/src/semmle/python/web/bottle/Request.qll b/python/ql/src/semmle/python/web/bottle/Request.qll new file mode 100644 index 000000000000..4039a1aa0bd6 --- /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(name)` as it wouldn't bind `name` */ + exists(string name | + fromnode = tonode.(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 = "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..715f3448a7a6 --- /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 bottle.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("response") +} + +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 UntrustedStringKind + } + +} + +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/Routing.expected b/python/ql/test/library-tests/web/bottle/Routing.expected new file mode 100644 index 000000000000..f4006d1caac3 --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Routing.expected @@ -0,0 +1,7 @@ +| /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 | +| /xss | test.py:35:1:35:16 | Function maybe_xss | 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/Sinks.expected b/python/ql/test/library-tests/web/bottle/Sinks.expected new file mode 100644 index 000000000000..64edd29140fe --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Sinks.expected @@ -0,0 +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/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/Sources.expected b/python/ql/test/library-tests/web/bottle/Sources.expected new file mode 100644 index 000000000000..ae780564c374 --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Sources.expected @@ -0,0 +1,10 @@ +| ../../../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 | +| 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/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/Taint.expected b/python/ql/test/library-tests/web/bottle/Taint.expected new file mode 100644 index 000000000000..ea97dab3473a --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/Taint.expected @@ -0,0 +1,25 @@ +| ../../../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 | +| 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 | +| 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 | +| 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/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() + 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..8975de72ea43 --- /dev/null +++ b/python/ql/test/library-tests/web/bottle/test.py @@ -0,0 +1,36 @@ + + +from bottle import Bottle, route, request, redirect, response + +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 + + +@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) + +@route('/xss') +def maybe_xss(): + response.body = "name is " + request.query.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..bd2d736c192e --- /dev/null +++ b/python/ql/test/query-tests/Security/lib/bottle.py @@ -0,0 +1,70 @@ + +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() + + +def redirect(url, code=None): + pass +