Merge branch 'jorgectf/python/headerInjection' into jorgectf/python/insecure-cookie

Author: jorgectf

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

@@ -213,21 +213,26 @@ class SQLEscape extends DataFlow::Node {
 /** Provides classes for modeling HTTP Header APIs. */
 module HeaderDeclaration {
   /**
-   * A data-flow node that collects functions setting HTTP Headers' content.
+   * A data-flow node that collects functions setting HTTP Headers.
    *
    * Extend this class to model new APIs. If you want to refine existing API models,
    * extend `HeaderDeclaration` instead.
    */
   abstract class Range extends DataFlow::Node {
+    /**
+     * Gets the argument containing the header name.
+     */
+    abstract DataFlow::Node getNameArg();
+
     /**
      * Gets the argument containing the header value.
      */
-    abstract DataFlow::Node getAnInput();
+    abstract DataFlow::Node getValueArg();
   }
 }
 
 /**
- * A data-flow node that collects functions setting HTTP Headers' content.
+ * A data-flow node that collects functions setting HTTP Headers.
  *
  * Extend this class to model new APIs. If you want to refine existing API models,
  * extend `HeaderDeclaration` instead.
@@ -238,7 +243,12 @@ class HeaderDeclaration extends DataFlow::Node {
   HeaderDeclaration() { this = range }
 
   /**
-   * Gets the argument containing the header value.
+   * Gets the argument containing the header name.
    */
-  DataFlow::Node getAnInput() { result = range.getAnInput() }
+  DataFlow::Node getNameArg() { result = range.getNameArg() }
+
+  /**
+   * Gets the argument containing the header name.
+   */
+  DataFlow::Node getValueArg() { result = range.getValueArg() }
 }

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

@@ -56,7 +56,9 @@ private module PrivateDjango {
           class DjangoResponseSetItemCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
             DjangoResponseSetItemCall() { this.getFunction() = headerSetItemCall() }
 
-            override DataFlow::Node getAnInput() { result = this.getArg([0, 1]) }
+            override DataFlow::Node getNameArg() { result = this.getArg(0) }
+
+            override DataFlow::Node getValueArg() { result = this.getArg(1) }
           }
 
           class DjangoResponseDefinition extends DataFlow::Node, HeaderDeclaration::Range {
@@ -67,9 +69,11 @@ private module PrivateDjango {
               headerInput.asCfgNode() = this.asCfgNode().(DefinitionNode).getValue()
             }
 
-            override DataFlow::Node getAnInput() {
-              result.asExpr() in [headerInput.asExpr(), this.asExpr().(Subscript).getIndex()]
+            override DataFlow::Node getNameArg() {
+              result.asExpr() = this.asExpr().(Subscript).getIndex()
             }
+
+            override DataFlow::Node getValueArg() { result = headerInput }
           }
         }
       }

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

@@ -54,20 +54,31 @@ module ExperimentalFlask {
       headerInput.asCfgNode() = this.asCfgNode().(DefinitionNode).getValue()
     }
 
-    override DataFlow::Node getAnInput() {
-      result.asExpr() in [headerInput.asExpr(), this.asExpr().(Subscript).getIndex()]
-    }
+    override DataFlow::Node getNameArg() { result.asExpr() = this.asExpr().(Subscript).getIndex() }
+
+    override DataFlow::Node getValueArg() { result = headerInput }
   }
 
   private class FlaskMakeResponseExtend extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
-    FlaskMakeResponseExtend() { this.getFunction() = headerInstanceCall() }
+    KeyValuePair item;
+
+    FlaskMakeResponseExtend() {
+      this.getFunction() = headerInstanceCall() and
+      item = this.getArg(_).asExpr().(Dict).getAnItem()
+    }
 
-    override DataFlow::Node getAnInput() { result = this.getArg(_) }
+    override DataFlow::Node getNameArg() { result.asExpr() = item.getKey() }
+
+    override DataFlow::Node getValueArg() { result.asExpr() = item.getValue() }
   }
 
   private class FlaskResponse extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
+    KeyValuePair item;
+
     FlaskResponse() { this = Flask::Response::classRef().getACall() }
 
-    override DataFlow::Node getAnInput() { result = this.getArgByName("headers") }
+    override DataFlow::Node getNameArg() { result.asExpr() = item.getKey() }
+
+    override DataFlow::Node getValueArg() { result.asExpr() = item.getValue() }
   }
 }

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

@@ -24,7 +24,9 @@ private module Werkzeug {
           this.getFunction().(DataFlow::AttrRead).getAttributeName() = "add"
         }
 
-        override DataFlow::Node getAnInput() { result = this.getArg(_) }
+        override DataFlow::Node getNameArg() { result = this.getArg(0) }
+
+        override DataFlow::Node getValueArg() { result = this.getArg(1) }
       }
     }
   }

python/ql/src/experimental/semmle/python/security/injection/HTTPHeaders.qll

@@ -13,6 +13,8 @@ class HeaderInjectionFlowConfig extends TaintTracking::Configuration {
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
   override predicate isSink(DataFlow::Node sink) {
-    sink = any(HeaderDeclaration headerDeclaration).getAnInput()
+    exists(HeaderDeclaration headerDeclaration |
+      sink in [headerDeclaration.getNameArg(), headerDeclaration.getValueArg()]
+    )
   }
 }