Merge pull request #2603 from RasmusWL/python-fix-http-source-sink

Python: Make web libs use HttpRequestTaintSource and HttpResponseTaintSink

Author: tausbn

change-notes/1.24/analysis-python.md

@@ -0,0 +1,37 @@
+# Improvements to Python analysis
+
+The following changes in version 1.24 affect Python analysis in all applications.
+
+## General improvements
+
+## New queries
+
+| **Query**                   | **Tags**  | **Purpose**                                                        |
+|-----------------------------|-----------|--------------------------------------------------------------------|
+
+## Changes to existing queries
+
+| **Query**                  | **Expected impact**    | **Change**                                                       |
+|----------------------------|------------------------|------------------------------------------------------------------|
+
+### Web framework support
+
+The QL-library support for the web frameworks Bottle, CherryPy, Falcon, Pyramid, TurboGears, Tornado, and Twisted have
+been fixed so they provide a proper HttpRequestTaintSource, instead of a TaintSource. This will enable results for the following queries:
+
+- py/path-injection
+- py/command-line-injection
+- py/reflective-xss
+- py/sql-injection
+- py/code-injection
+- py/unsafe-deserialization
+- py/url-redirection
+
+The QL-library support for the web framework Twisted have been fixed so they provide a proper
+HttpResponseTaintSink, instead of a TaintSink. This will enable results for the following
+queries:
+
+- py/reflective-xss
+- py/stack-trace-exposure
+
+## Changes to libraries

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

@@ -21,7 +21,7 @@ class BottleRequestKind extends TaintKind {
     }
 }
 
-private class RequestSource extends TaintSource {
+private class RequestSource extends HttpRequestTaintSource {
     RequestSource() { this.(ControlFlowNode).pointsTo(theBottleRequestObject()) }
 
     override predicate isSourceOf(TaintKind kind) { kind instanceof BottleRequestKind }
@@ -69,7 +69,7 @@ class UntrustedFile extends TaintKind {
 //  Move UntrustedFile to shared location
 //
 /** Parameter to a bottle request handler function */
-class BottleRequestParameter extends TaintSource {
+class BottleRequestParameter extends HttpRequestTaintSource {
     BottleRequestParameter() {
         exists(BottleRoute route | route.getNamedArgument() = this.(ControlFlowNode).getNode())
     }

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

@@ -25,7 +25,7 @@ class CherryPyRequest extends TaintKind {
     }
 }
 
-class CherryPyExposedFunctionParameter extends TaintSource {
+class CherryPyExposedFunctionParameter extends HttpRequestTaintSource {
     CherryPyExposedFunctionParameter() {
         exists(Parameter p |
             p = any(CherryPyExposedFunction f).getAnArg() and
@@ -39,7 +39,7 @@ class CherryPyExposedFunctionParameter extends TaintSource {
     override predicate isSourceOf(TaintKind kind) { kind instanceof ExternalStringKind }
 }
 
-class CherryPyRequestSource extends TaintSource {
+class CherryPyRequestSource extends HttpRequestTaintSource {
     CherryPyRequestSource() { this.(ControlFlowNode).pointsTo(Value::named("cherrypy.request")) }
 
     override predicate isSourceOf(TaintKind kind) { kind instanceof CherryPyRequest }

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

@@ -18,8 +18,8 @@ private ClassValue theDjangoHttpResponseClass() {
     not result = theDjangoHttpRedirectClass()
 }
 
-/** Instantiation of a django response. */
-class DjangoResponseSource extends TaintSource {
+/** internal class used for tracking a django response. */
+private class DjangoResponseSource extends TaintSource {
     DjangoResponseSource() {
         exists(ClassValue cls |
             cls.getASuperType() = theDjangoHttpResponseClass() and

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

@@ -35,7 +35,7 @@ class FalconRequest extends TaintKind {
     }
 }
 
-class FalconRequestParameter extends TaintSource {
+class FalconRequestParameter extends HttpRequestTaintSource {
     FalconRequestParameter() {
         exists(FalconHandlerFunction f | f.getRequest() = this.(ControlFlowNode).getNode())
     }

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

@@ -9,7 +9,8 @@ class FalconResponse extends TaintKind {
     FalconResponse() { this = "falcon.response" }
 }
 
-class FalconResponseParameter extends TaintSource {
+/** Only used internally to track the response parameter */
+private class FalconResponseParameter extends TaintSource {
     FalconResponseParameter() {
         exists(FalconHandlerFunction f | f.getResponse() = this.(ControlFlowNode).getNode())
     }

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

@@ -47,7 +47,7 @@ class FlaskRequestArgs extends HttpRequestTaintSource {
 }
 
 /** Source of dictionary whose values are externally controlled */
-class FlaskRequestJson extends TaintSource {
+class FlaskRequestJson extends HttpRequestTaintSource {
     FlaskRequestJson() { flask_request_attr(this, "json") }
 
     override predicate isSourceOf(TaintKind kind) { kind instanceof ExternalJsonKind }

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

@@ -11,7 +11,7 @@ class PyramidRequest extends BaseWebobRequest {
 }
 
 /** Source of pyramid request objects */
-class PyramidViewArgument extends TaintSource {
+class PyramidViewArgument extends HttpRequestTaintSource {
     PyramidViewArgument() {
         exists(Function view_func |
             is_pyramid_view_function(view_func) and

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

@@ -13,14 +13,16 @@ import Tornado
 /**
  * Represents an argument to the `tornado.redirect` function.
  */
-class TornadoRedirect extends HttpRedirectTaintSink {
-    override string toString() { result = "tornado.redirect" }
+class TornadoHttpRequestHandlerRedirect extends HttpRedirectTaintSink {
+    override string toString() { result = "tornado.HttpRequestHandler.redirect" }
 
-    TornadoRedirect() {
+    TornadoHttpRequestHandlerRedirect() {
         exists(CallNode call, ControlFlowNode node |
             node = call.getFunction().(AttrNode).getObject("redirect") and
             isTornadoRequestHandlerInstance(node) and
-            this = call.getAnArg()
+            this = call.getArg(0)
         )
     }
+
+    override predicate sinks(TaintKind kind) { kind instanceof StringKind }
 }

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

@@ -30,15 +30,15 @@ class TornadoRequest extends TaintKind {
     }
 }
 
-class TornadoRequestSource extends TaintSource {
+class TornadoRequestSource extends HttpRequestTaintSource {
     TornadoRequestSource() { isTornadoRequestHandlerInstance(this.(AttrNode).getObject("request")) }
 
     override string toString() { result = "Tornado request source" }
 
     override predicate isSourceOf(TaintKind kind) { kind instanceof TornadoRequest }
 }
 
-class TornadoExternalInputSource extends TaintSource {
+class TornadoExternalInputSource extends HttpRequestTaintSource {
     TornadoExternalInputSource() {
         exists(string name |
             name = "get_argument" or
@@ -55,7 +55,7 @@ class TornadoExternalInputSource extends TaintSource {
     override predicate isSourceOf(TaintKind kind) { kind instanceof ExternalStringKind }
 }
 
-class TornadoExternalInputListSource extends TaintSource {
+class TornadoExternalInputListSource extends HttpRequestTaintSource {
     TornadoExternalInputListSource() {
         exists(string name |
             name = "get_arguments" or