Apply structure

python/ql/src/experimental/Security/CWE-113/HeaderInjection.ql

@@ -1,10 +1,10 @@
 /**
  * @name HTTP Header Injection
- * @description User input should not be used in HTTP headers without first being escaped,
- *              otherwise a malicious user may be able to inject a value that could manipulate the response.
+ * @description User input should not be used in HTTP headers, otherwise a malicious user
+ *              may be able to inject a value that could manipulate the response.
  * @kind path-problem
  * @problem.severity error
- * @id python/header-injection
+ * @id py/header-injection
  * @tags security
  *       external/cwe/cwe-113
  *       external/cwe/cwe-079
@@ -12,152 +12,10 @@
 
 // determine precision above
 import python
-import semmle.python.dataflow.new.RemoteFlowSources
-import semmle.python.dataflow.new.DataFlow
-import semmle.python.dataflow.new.TaintTracking
-import semmle.python.ApiGraphs
+import experimental.semmle.python.security.injection.HTTPHeaders
 import DataFlow::PathGraph
 
-class WerkzeugHeaderCall extends DataFlow::CallCfgNode {
-  WerkzeugHeaderCall() {
-    exists(DataFlow::AttrRead addMethod |
-      this.getFunction() = addMethod and
-      addMethod.getObject().getALocalSource() =
-        API::moduleImport("werkzeug").getMember("datastructures").getMember("Headers").getACall() and
-      addMethod.getAttributeName() = "add"
-    )
-  }
-
-  DataFlow::Node getHeaderInputNode() { result = this.getArg(1) }
-}
-
-class FlaskHeaderCall extends DataFlow::Node {
-  DataFlow::Node headerInputNode;
-
-  FlaskHeaderCall() {
-    exists(
-      DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod,
-      AssignStmt sinkDeclaration
-    |
-      headerInstance = API::moduleImport("flask").getMember("Response").getACall() and
-      responseMethod.getAttributeName() = "headers" and
-      responseMethod.getObject().getALocalSource() = headerInstance and
-      sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and
-      headerInputNode.asExpr() = sinkDeclaration.getValue() and
-      this.asExpr() = sinkDeclaration.getATarget()
-    )
-  }
-
-  DataFlow::Node getHeaderInputNode() { result = headerInputNode }
-}
-
-class FlaskMakeResponseCall extends DataFlow::Node {
-  DataFlow::Node headerInputNode;
-
-  FlaskMakeResponseCall() {
-    exists(
-      DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod,
-      AssignStmt sinkDeclaration
-    |
-      headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and
-      responseMethod.getAttributeName() = "headers" and
-      responseMethod.getObject().getALocalSource() = headerInstance and
-      sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and
-      this.asExpr() = sinkDeclaration.getATarget() and
-      headerInputNode.asExpr() = sinkDeclaration.getValue()
-    )
-  }
-
-  DataFlow::Node getHeaderInputNode() { result = headerInputNode }
-}
-
-class FlaskMakeResponseExtendCall extends DataFlow::CallCfgNode {
-  DataFlow::Node headerInputNode;
-
-  FlaskMakeResponseExtendCall() {
-    exists(
-      DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod,
-      DataFlow::AttrRead extendMethod
-    |
-      headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and
-      responseMethod.getAttributeName() = "headers" and
-      responseMethod.getObject().getALocalSource() = headerInstance and
-      extendMethod.getAttributeName() = "extend" and
-      extendMethod.getObject().getALocalSource() = responseMethod and
-      this.getFunction() = extendMethod and
-      headerInputNode = this.getArg(0)
-    )
-  }
-
-  DataFlow::Node getHeaderInputNode() { result = headerInputNode }
-}
-
-class FlaskResponseArg extends DataFlow::CallCfgNode {
-  DataFlow::Node headerInputNode;
-
-  FlaskResponseArg() {
-    this = API::moduleImport("flask").getMember("Response").getACall() and
-    headerInputNode = this.getArgByName("headers")
-  }
-
-  DataFlow::Node getHeaderInputNode() { result = headerInputNode }
-}
-
-class DjangoResponseSetItemCall extends DataFlow::CallCfgNode {
-  DjangoResponseSetItemCall() {
-    exists(DataFlow::AttrRead setItemMethod |
-      this.getFunction() = setItemMethod and
-      setItemMethod.getObject().getALocalSource() =
-        API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and
-      setItemMethod.getAttributeName() = "__setitem__"
-    )
-  }
-
-  DataFlow::Node getHeaderInputNode() { result = this.getArg(1) }
-}
-
-class DjangoResponseAssignCall extends DataFlow::Node {
-  DataFlow::Node headerInputNode;
-
-  DjangoResponseAssignCall() {
-    exists(
-      DataFlow::CallCfgNode headerInstance, Subscript responseMethod, DataFlow::Node responseToNode,
-      AssignStmt sinkDeclaration
-    |
-      headerInstance =
-        API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and
-      responseMethod.getValue() = responseToNode.asExpr() and
-      responseToNode.getALocalSource().asExpr() = headerInstance.asExpr() and
-      sinkDeclaration.getATarget() = responseMethod and
-      this.asExpr() = sinkDeclaration.getATarget() and
-      headerInputNode.asExpr() = sinkDeclaration.getValue()
-    )
-  }
-
-  DataFlow::Node getHeaderInputNode() { result = headerInputNode }
-}
-
-class HeaderInjectionSink extends DataFlow::Node {
-  HeaderInjectionSink() {
-    this = any(WerkzeugHeaderCall a).getHeaderInputNode() or
-    this = any(FlaskHeaderCall a).getHeaderInputNode() or
-    this = any(FlaskMakeResponseCall a).getHeaderInputNode() or
-    this = any(FlaskMakeResponseExtendCall a).getHeaderInputNode() or
-    this = any(FlaskResponseArg a).getHeaderInputNode() or
-    this = any(DjangoResponseSetItemCall a).getHeaderInputNode() or
-    this = any(DjangoResponseAssignCall a).getHeaderInputNode()
-  }
-}
-
-class HeaderInjectionFlowConfig extends TaintTracking::Configuration {
-  HeaderInjectionFlowConfig() { this = "HeaderInjectionFlowConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof HeaderInjectionSink }
-}
-
 from HeaderInjectionFlowConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "$@ header is constructed from a $@.", sink.getNode(), "This",
-  source.getNode(), "user-provided value"
+select sink.getNode(), source, sink, "$@ HTTP header is constructed from a $@.", sink.getNode(),
+  "This", source.getNode(), "user-provided value"

python/ql/src/experimental/Security/CWE-113/tests/django_bad.py

@@ -1,15 +0,0 @@
-import django.http
-
-
-def django_setitem(request):
-    rfs_header = request.GET.get("rfs_header")
-    response = django.http.HttpResponse()
-    response.__setitem__('HeaderName', rfs_header)
-    return response
-
-
-def django_response(request):
-    rfs_header = request.GET.get("rfs_header")
-    response = django.http.HttpResponse()
-    response['HeaderName'] = rfs_header
-    return response

python/ql/src/experimental/Security/CWE-113/tests/flask_bad.py

@@ -1,47 +0,0 @@
-from flask import Response, request, Flask, make_response
-from werkzeug.datastructures import Headers
-
-app = Flask(__name__)
-
-
-@app.route('/werkzeug_headers')
-def werkzeug_headers():
-    rfs_header = request.args["rfs_header"]
-    response = Response()
-    headers = Headers()
-    headers.add("HeaderName", rfs_header)
-    response.headers = headers
-    return response
-
-
-@app.route("/flask_Response")
-def flask_Response():
-    rfs_header = request.args["rfs_header"]
-    response = Response()
-    response.headers['HeaderName'] = rfs_header
-    return response
-
-
-@app.route("/flask_make_response")
-def flask_make_response():
-    rfs_header = request.args["rfs_header"]
-    resp = make_response("hello")
-    resp.headers['HeaderName'] = rfs_header
-    return resp
-
-
-@app.route("/flask_make_response_extend")
-def flask_make_response_extend():
-    rfs_header = request.args["rfs_header"]
-    resp = make_response("hello")
-    resp.headers.extend(
-        {'HeaderName': rfs_header})
-    return resp
-
-
-@app.route("/Response_arg")
-def Response_arg():
-    return Response(headers={'HeaderName': request.args["rfs_header"]})
-
-# if __name__ == "__main__":
-#     app.run(debug=True)

python/ql/src/experimental/semmle/python/Concepts.qll

@@ -13,3 +13,17 @@ private import semmle.python.dataflow.new.DataFlow
 private import semmle.python.dataflow.new.RemoteFlowSources
 private import semmle.python.dataflow.new.TaintTracking
 private import experimental.semmle.python.Frameworks
+
+module HeaderDeclaration {
+  abstract class Range extends DataFlow::Node {
+    abstract DataFlow::Node getHeaderInputNode();
+  }
+}
+
+class HeaderDeclaration extends DataFlow::Node {
+  HeaderDeclaration::Range range;
+
+  HeaderDeclaration() { this = range }
+
+  DataFlow::Node getHeaderInputNode() { result = range.getHeaderInputNode() }
+}

python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll

@@ -9,3 +9,133 @@ private import semmle.python.dataflow.new.TaintTracking
 private import semmle.python.dataflow.new.RemoteFlowSources
 private import experimental.semmle.python.Concepts
 private import semmle.python.ApiGraphs
+
+private module Headers {
+  private module Werkzeug {
+    class WerkzeugHeaderCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
+      WerkzeugHeaderCall() {
+        exists(DataFlow::AttrRead addMethod |
+          this.getFunction() = addMethod and
+          addMethod.getObject().getALocalSource() =
+            API::moduleImport("werkzeug")
+                .getMember("datastructures")
+                .getMember("Headers")
+                .getACall() and
+          addMethod.getAttributeName() = "add"
+        )
+      }
+
+      override DataFlow::Node getHeaderInputNode() { result = this.getArg(1) }
+    }
+  }
+
+  private module Flask {
+    class FlaskHeaderCall extends DataFlow::Node, HeaderDeclaration::Range {
+      DataFlow::Node headerInputNode;
+
+      FlaskHeaderCall() {
+        exists(
+          DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod,
+          AssignStmt sinkDeclaration
+        |
+          headerInstance = API::moduleImport("flask").getMember("Response").getACall() and
+          responseMethod.getAttributeName() = "headers" and
+          responseMethod.getObject().getALocalSource() = headerInstance and
+          sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and
+          headerInputNode.asExpr() = sinkDeclaration.getValue() and
+          this.asExpr() = sinkDeclaration.getATarget()
+        )
+      }
+
+      override DataFlow::Node getHeaderInputNode() { result = headerInputNode }
+    }
+
+    class FlaskMakeResponseCall extends DataFlow::Node, HeaderDeclaration::Range {
+      DataFlow::Node headerInputNode;
+
+      FlaskMakeResponseCall() {
+        exists(
+          DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod,
+          AssignStmt sinkDeclaration
+        |
+          headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and
+          responseMethod.getAttributeName() = "headers" and
+          responseMethod.getObject().getALocalSource() = headerInstance and
+          sinkDeclaration.getATarget() = responseMethod.asExpr().getParentNode() and
+          this.asExpr() = sinkDeclaration.getATarget() and
+          headerInputNode.asExpr() = sinkDeclaration.getValue()
+        )
+      }
+
+      override DataFlow::Node getHeaderInputNode() { result = headerInputNode }
+    }
+
+    class FlaskMakeResponseExtendCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
+      DataFlow::Node headerInputNode;
+
+      FlaskMakeResponseExtendCall() {
+        exists(
+          DataFlow::CallCfgNode headerInstance, DataFlow::AttrRead responseMethod,
+          DataFlow::AttrRead extendMethod
+        |
+          headerInstance = API::moduleImport("flask").getMember("make_response").getACall() and
+          responseMethod.getAttributeName() = "headers" and
+          responseMethod.getObject().getALocalSource() = headerInstance and
+          extendMethod.getAttributeName() = "extend" and
+          extendMethod.getObject().getALocalSource() = responseMethod and
+          this.getFunction() = extendMethod and
+          headerInputNode = this.getArg(0)
+        )
+      }
+
+      override DataFlow::Node getHeaderInputNode() { result = headerInputNode }
+    }
+
+    class FlaskResponseArg extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
+      DataFlow::Node headerInputNode;
+
+      FlaskResponseArg() {
+        this = API::moduleImport("flask").getMember("Response").getACall() and
+        headerInputNode = this.getArgByName("headers")
+      }
+
+      override DataFlow::Node getHeaderInputNode() { result = headerInputNode }
+    }
+
+    class DjangoResponseSetItemCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
+      DjangoResponseSetItemCall() {
+        exists(DataFlow::AttrRead setItemMethod |
+          this.getFunction() = setItemMethod and
+          setItemMethod.getObject().getALocalSource() =
+            API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and
+          setItemMethod.getAttributeName() = "__setitem__"
+        )
+      }
+
+      override DataFlow::Node getHeaderInputNode() { result = this.getArg(1) }
+    }
+  }
+
+  private module Django {
+    class DjangoResponseAssignCall extends DataFlow::Node, HeaderDeclaration::Range {
+      DataFlow::Node headerInputNode;
+
+      DjangoResponseAssignCall() {
+        exists(
+          DataFlow::CallCfgNode headerInstance, Subscript responseMethod,
+          DataFlow::Node responseToNode, AssignStmt sinkDeclaration
+        |
+          headerInstance =
+            API::moduleImport("django").getMember("http").getMember("HttpResponse").getACall() and
+          responseMethod.getValue() = responseToNode.asExpr() and
+          responseToNode.getALocalSource().asExpr() = headerInstance.asExpr() and
+          sinkDeclaration.getATarget() = responseMethod and
+          this.asExpr() = sinkDeclaration.getATarget() and
+          headerInputNode.asExpr() = sinkDeclaration.getValue()
+        )
+      }
+
+      override DataFlow::Node getHeaderInputNode() { result = headerInputNode }
+    }
+  }
+}