Merge pull request #2459 from RasmusWL/python-modernise-TurboGears-library

Python: modernise TurboGears library

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

@@ -1,21 +1,17 @@
 import python
 import semmle.python.security.strings.Untrusted
-
 import TurboGears
 
 private class ValidatedMethodParameter extends Parameter {
-
     ValidatedMethodParameter() {
         exists(string name, TurboGearsControllerMethod method |
             method.getArgByName(name) = this and
             method.getValidationDict().getItem(_).(KeyValuePair).getKey().(StrConst).getText() = name
         )
     }
-
 }
 
 class UnvalidatedControllerMethodParameter extends TaintSource {
-
     UnvalidatedControllerMethodParameter() {
         exists(Parameter p |
             any(TurboGearsControllerMethod m | not m.getName() = "onerror").getAnArg() = p and
@@ -25,9 +21,5 @@ class UnvalidatedControllerMethodParameter extends TaintSource {
         )
     }
 
-    override predicate isSourceOf(TaintKind kind) {
-        kind instanceof UntrustedStringKind
-    }
-
+    override predicate isSourceOf(TaintKind kind) { kind instanceof UntrustedStringKind }
 }
-

python/ql/src/semmle/python/web/turbogears/Response.qll

@@ -1,14 +1,10 @@
 import python
-
 import semmle.python.security.TaintTracking
 import semmle.python.security.strings.Basic
 import semmle.python.web.Http
 import TurboGears
 
-
-
 class ControllerMethodReturnValue extends HttpResponseTaintSink {
-
     ControllerMethodReturnValue() {
         exists(TurboGearsControllerMethod m |
             m.getAReturnValueFlowNode() = this and
@@ -16,14 +12,10 @@ class ControllerMethodReturnValue extends HttpResponseTaintSink {
         )
     }
 
-    override predicate sinks(TaintKind kind) {
-        kind instanceof StringKind
-    }
-
+    override predicate sinks(TaintKind kind) { kind instanceof StringKind }
 }
 
 class ControllerMethodTemplatedReturnValue extends HttpResponseTaintSink {
-
     ControllerMethodTemplatedReturnValue() {
         exists(TurboGearsControllerMethod m |
             m.getAReturnValueFlowNode() = this and
@@ -31,8 +23,5 @@ class ControllerMethodTemplatedReturnValue extends HttpResponseTaintSink {
         )
     }
 
-    override predicate sinks(TaintKind kind) {
-        kind instanceof StringDictKind
-    }
-
+    override predicate sinks(TaintKind kind) { kind instanceof StringDictKind }
 }

python/ql/src/semmle/python/web/turbogears/TurboGears.qll

@@ -1,55 +1,33 @@
 import python
-
 import semmle.python.security.TaintTracking
 
-private ClassObject theTurboGearsControllerClass() {
-    result = ModuleObject::named("tg").attr("TGController")
-}
-
-
-ClassObject aTurboGearsControllerClass() {
-    result.getASuperType() = theTurboGearsControllerClass()
-}
+private ClassValue theTurboGearsControllerClass() { result = Value::named("tg.TGController") }
 
+ClassValue aTurboGearsControllerClass() { result.getABaseType+() = theTurboGearsControllerClass() }
 
 class TurboGearsControllerMethod extends Function {
-
     ControlFlowNode decorator;
 
     TurboGearsControllerMethod() {
-        aTurboGearsControllerClass().getPyClass() = this.getScope() and
+        aTurboGearsControllerClass().getScope() = this.getScope() and
         decorator = this.getADecorator().getAFlowNode() and
         /* Is decorated with @expose() or @expose(path) */
         (
             decorator.(CallNode).getFunction().(NameNode).getId() = "expose"
             or
-            decorator.refersTo(_, ModuleObject::named("tg").attr("expose"), _)
+            decorator.pointsTo().getClass() = Value::named("tg.expose")
         )
     }
 
-    private ControlFlowNode templateName() {
-        result = decorator.(CallNode).getArg(0)
-    }
+    private ControlFlowNode templateName() { result = decorator.(CallNode).getArg(0) }
 
-    predicate isTemplated() {
-        exists(templateName())
-    }
-
-    string getTemplateName() {
-        exists(StringObject str |
-            templateName().refersTo(str) and
-            result = str.getText()
-        )
-    }
+    predicate isTemplated() { exists(templateName()) }
 
     Dict getValidationDict() {
-        exists(Call call, Object dict |
+        exists(Call call, Value dict |
             call = this.getADecorator() and
             call.getFunc().(Name).getId() = "validate" and
-            call.getArg(0).refersTo(dict) and
-            result = dict.getOrigin()
+            call.getArg(0).pointsTo(dict, result)
         )
     }
-
 }
-

python/ql/test/library-tests/web/turbogears/Controller.expected

@@ -2,3 +2,4 @@
 | test.py:13:5:13:50 | Function ok_validated |
 | test.py:18:5:18:57 | Function partially_validated |
 | test.py:22:5:22:51 | Function not_validated |
+| test.py:26:5:26:28 | Function with_template |

python/ql/test/library-tests/web/turbogears/Sinks.expected

@@ -2,3 +2,4 @@
 | test.py:14 | BinaryExpr | externally controlled string |
 | test.py:19 | BinaryExpr | externally controlled string |
 | test.py:23 | BinaryExpr | externally controlled string |
+| test.py:27 | Dict | {externally controlled string} |

python/ql/test/library-tests/web/turbogears/test.py

@@ -21,3 +21,7 @@ class RootController(TGController):
     @expose()
     def not_validated(self, a=None, b=None, *args):
         return 'Values: %s, %s, %s' % (a, b, args)
+
+    @expose("<template_path>")
+    def with_template(self):
+        return {'template_var': 'foo'}