Merge pull request #872 from markshannon/python-bottle

Python: Add support for bottle framework.

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.
+ 

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

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

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

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)
+    }
+}
+

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
+    }
+
+}

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"
+    }
+
+}
+

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"
+    }
+
+}
+

python/ql/test/library-tests/web/bottle/Routing.expected

@@ -0,0 +1,7 @@
+| /args | test.py:31:1:31:14 | Function unsafe2 |
+| /bye/<name> | test.py:12:1:12:25 | Function bye |
+| /hello/<name> | test.py:8:1:8:27 | Function hello |
+| /other | test.py:17:1:17:12 | Function other |
+| /wrong/<where> | 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 |

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()

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 |

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

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 |

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

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 |

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()
+

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

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/<name>')
+def hello(name = "World!"):
+    return "Hello " + name
+
+@route('/bye/<name>')
+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/<where>')
+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

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
+