Merge pull request #2034 from RasmusWL/python-modernise-bottle

Python: modernise bottle

Author: taus-semmle

python/ql/src/semmle/python/web/bottle/General.qll

@@ -3,23 +3,20 @@ import semmle.python.web.Http
 import semmle.python.types.Extensions
 
 /** The bottle module */
-ModuleObject theBottleModule() {
-    result = ModuleObject::named("bottle")
-}
+ModuleValue theBottleModule() { result = Module::named("bottle") }
 
 /** The bottle.Bottle class */
-ClassObject theBottleClass() {
-    result = theBottleModule().attr("Bottle")
-}
+ClassValue theBottleClass() { result = theBottleModule().attr("Bottle") }
 
-/** Holds if `route` is routed to `func`
+/**
+ * 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().attr(name))
-        |
+        route_call.getFunction().(AttrNode).getObject(name).pointsTo().getClass() = theBottleClass() or
+        route_call.getFunction().pointsTo(theBottleModule().attr(name))
+    |
         (name = "route" or name = httpVerbLower()) and
         decorator_call.getFunction() = route_call and
         route_call.getArg(0) = route and
@@ -28,10 +25,7 @@ predicate bottle_route(CallNode route_call, ControlFlowNode route, Function func
 }
 
 class BottleRoute extends ControlFlowNode {
-
-    BottleRoute() {
-        bottle_route(this, _, _)
-    }
+    BottleRoute() { bottle_route(this, _, _) }
 
     string getUrl() {
         exists(StrConst url |
@@ -40,9 +34,7 @@ class BottleRoute extends ControlFlowNode {
         )
     }
 
-    Function getFunction() {
-        bottle_route(this, _, result)
-    }
+    Function getFunction() { bottle_route(this, _, result) }
 
     Parameter getNamedArgument() {
         exists(string name, Function func |
@@ -52,5 +44,3 @@ class BottleRoute extends ControlFlowNode {
         )
     }
 }
-
-

python/ql/src/semmle/python/web/bottle/Redirect.qll

@@ -1,25 +1,21 @@
-/** Provides class representing the `bottle.redirect` function.
+/**
+ * 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 python
 import semmle.python.security.TaintTracking
 import semmle.python.security.strings.Basic
 import semmle.python.web.bottle.General
 
-FunctionObject bottle_redirect() {
-    result = theBottleModule().attr("redirect")
-}
+FunctionValue bottle_redirect() { result = theBottleModule().attr("redirect") }
 
 /**
  * Represents an argument to the `bottle.redirect` function.
  */
 class BottleRedirect extends TaintSink {
-
-    override string toString() {
-        result = "bottle.redirect"
-    }
+    override string toString() { result = "bottle.redirect" }
 
     BottleRedirect() {
         exists(CallNode call |
@@ -28,8 +24,5 @@ class BottleRedirect extends TaintSink {
         )
     }
 
-    override predicate sinks(TaintKind kind) {
-        kind instanceof StringKind
-    }
-
+    override predicate sinks(TaintKind kind) { kind instanceof StringKind }
 }

python/ql/src/semmle/python/web/bottle/Request.qll

@@ -1,20 +1,13 @@
 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().attr("request")
-}
+private Value theBottleRequestObject() { result = theBottleModule().attr("request") }
 
 class BottleRequestKind extends TaintKind {
-
-    BottleRequestKind() {
-        this = "bottle.request"
-    }
+    BottleRequestKind() { this = "bottle.request" }
 
     override TaintKind getTaintOfAttribute(string name) {
         result instanceof BottleFormsDict and
@@ -26,34 +19,23 @@ class BottleRequestKind extends TaintKind {
         result.(DictKind).getValue() instanceof FileUpload and
         name = "files"
     }
-
 }
 
 private class RequestSource extends TaintSource {
+    RequestSource() { this.(ControlFlowNode).pointsTo(theBottleRequestObject()) }
 
-    RequestSource() {
-        this.(ControlFlowNode).refersTo(theBottleRequestObject())
-    }
-
-    override predicate isSourceOf(TaintKind kind) {
-        kind instanceof BottleRequestKind
-    }
-
+    override predicate isSourceOf(TaintKind kind) { kind instanceof BottleRequestKind }
 }
 
-
 class BottleFormsDict extends TaintKind {
-
-    BottleFormsDict() {
-        this = "bottle.FormsDict"
-    }
+    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 
+            fromnode = tonode.(AttrNode).getObject(name) and
             result instanceof UntrustedStringKind
-            |
+        |
             name != "get" and name != "getunicode" and name != "getall"
         )
     }
@@ -67,10 +49,7 @@ class BottleFormsDict extends TaintKind {
 }
 
 class FileUpload extends TaintKind {
-
-    FileUpload() {
-        this = "bottle.FileUpload"
-    }
+    FileUpload() { this = "bottle.FileUpload" }
 
     override TaintKind getTaintOfAttribute(string name) {
         name = "filename" and result instanceof UntrustedStringKind
@@ -79,37 +58,23 @@ class FileUpload extends TaintKind {
         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
+        exists(BottleRoute route | route.getNamedArgument() = this.(ControlFlowNode).getNode())
     }
 
-    override string toString() {
-        result = "bottle handler function argument"
-    }
+    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

@@ -1,63 +1,45 @@
 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
+/**
+ * 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"
-    }
-
+    BottleResponse() { this = "bottle.response" }
 }
 
-private Object theBottleResponseObject() {
-    result = theBottleModule().attr("response")
-}
+private Value theBottleResponseObject() { result = theBottleModule().attr("response") }
 
 class BottleResponseBodyAssignment extends HttpResponseTaintSink {
-
     BottleResponseBodyAssignment() {
         exists(DefinitionNode lhs |
             lhs.getValue() = this and
-            lhs.(AttrNode).getObject("body").refersTo(theBottleResponseObject())
+            lhs.(AttrNode).getObject("body").pointsTo(theBottleResponseObject())
         )
     }
 
-    override predicate sinks(TaintKind kind) {
-        kind instanceof StringKind
-    }
-
+    override predicate sinks(TaintKind kind) { kind instanceof StringKind }
 }
 
 class BottleHandlerFunctionResult extends HttpResponseTaintSink {
-
     BottleHandlerFunctionResult() {
         exists(BottleRoute route, Return ret |
             ret.getScope() = route.getFunction() and
             ret.getValue().getAFlowNode() = this
         )
     }
 
-    override predicate sinks(TaintKind kind) {
-        kind instanceof StringKind
-    }
-
-    override string toString() {
-        result = "bottle handler function result"
-    }
+    override predicate sinks(TaintKind kind) { kind instanceof StringKind }
 
+    override string toString() { result = "bottle handler function result" }
 }
 
 class BottleCookieSet extends CookieSet, CallNode {
-
     BottleCookieSet() {
         any(BottleResponse r).taints(this.getFunction().(AttrNode).getObject("set_cookie"))
     }
@@ -67,6 +49,4 @@ class BottleCookieSet extends CookieSet, CallNode {
     override ControlFlowNode getKey() { result = this.getArg(0) }
 
     override ControlFlowNode getValue() { result = this.getArg(1) }
-
 }
-

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

@@ -1,7 +1,5 @@
 | ../../../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 |

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

@@ -1,8 +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 |
 | test.py:9 | BinaryExpr | externally controlled string |
 | test.py:9 | name | externally controlled string |

python/ql/test/query-tests/Security/lib/bottle.py

@@ -67,4 +67,3 @@ class LocalResponse(LocalProxy):
 
 def redirect(url, code=None):
     pass
-