Add to header write concept a specification of whether the name or value arg allows newlines.

Ported sink defenitions from Flask and Werzeug from experimental to main.
Removed experimental sink definitions for Django, as neither name nor value are vulnerable.

Author: joefarebrother

python/ql/lib/semmle/python/Concepts.qll

@@ -1041,6 +1041,16 @@ module Http {
        * Gets the argument containing the header value.
        */
       DataFlow::Node getValueArg() { result = super.getValueArg() }
+
+      /**
+       * Holds if newlines are accepted in the header name argument.
+       */
+      predicate nameAllowsNewline() { super.nameAllowsNewline() }
+
+      /**
+       * Holds if newlines are accepted in the header value argument.
+       */
+      predicate valueAllowsNewline() { super.valueAllowsNewline() }
     }
 
     /** Provides a class for modelling header writes on HTTP responses. */
@@ -1061,6 +1071,16 @@ module Http {
          * Gets the argument containing the header value.
          */
         abstract DataFlow::Node getValueArg();
+
+        /**
+         * Holds if newlines are accepted in the header name argument.
+         */
+        abstract predicate nameAllowsNewline();
+
+        /**
+         * Holds if newlines are accepted in the header value argument.
+         */
+        abstract predicate valueAllowsNewline();
       }
     }
 

python/ql/lib/semmle/python/frameworks/Flask.qll

@@ -220,6 +220,13 @@ module Flask {
 
     /** Gets a reference to an instance of `flask.Response`. */
     DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
+
+    /** An `Headers` instance that is part of a Flask response. */
+    private class FlaskResponseHeadersInstances extends Werkzeug::Headers::InstanceSource {
+      FlaskResponseHeadersInstances() { this = request().getMember("headers").asSource() }
+    }
+
+    // TODO: headers arg to make_response
   }
 
   // ---------------------------------------------------------------------------

python/ql/lib/semmle/python/frameworks/Werkzeug.qll

@@ -182,8 +182,52 @@ module Werkzeug {
 
       override string getAsyncMethodName() { none() }
     }
+
+    /** A call to a method that writes to a header, assumed to be a response header. */
+    private class HeaderWriteCall extends Http::Server::ResponseHeaderWrite::Range,
+      DataFlow::MethodCallNode
+    {
+      HeaderWriteCall() {
+        this.getObject() = instance() and
+        this.getMethodName() = ["add", "add_header", "set", "set_default", "__setitem__"]
+      }
+
+      override DataFlow::Node getNameArg() { result = this.getArg(0) }
+
+      override DataFlow::Node getValueArg() { result = this.getArg(1) }
+
+      override predicate nameAllowsNewline() { any() }
+
+      override predicate valueAllowsNewline() { none() }
+    }
+
+    /** A dict-like write to a header, assumed to be a response header. */
+    private class HeaderWriteSubscript extends Http::Server::ResponseHeaderWrite::Range,
+      DataFlow::Node
+    {
+      DataFlow::Node name;
+      DataFlow::Node value;
+
+      HeaderWriteSubscript() {
+        exists(SubscriptNode subscript |
+          this.asCfgNode() = subscript and
+          value.asCfgNode() = subscript.(DefinitionNode).getValue() and
+          name.asCfgNode() = subscript.getIndex() and
+          subscript.getObject() = instance().asCfgNode()
+        )
+      }
+
+      override DataFlow::Node getNameArg() { result = name }
+
+      override DataFlow::Node getValueArg() { result = value }
+
+      override predicate nameAllowsNewline() { any() }
+
+      override predicate valueAllowsNewline() { none() }
+    }
   }
 
+  // TODO: `extend` bulk header update
   /**
    * Provides models for the `werkzeug.datastructures.Authorization` class
    *

python/ql/lib/semmle/python/security/dataflow/HttpHeaderInjectionCustomizations.qll

@@ -41,8 +41,12 @@ module HttpHeaderInjection {
    */
   class HeaderWriteAsSink extends Sink {
     HeaderWriteAsSink() {
-      exists(Http::Server::ResponseHeaderWrite headerDeclaration |
-        this in [headerDeclaration.getNameArg(), headerDeclaration.getValueArg()]
+      exists(Http::Server::ResponseHeaderWrite headerWrite |
+        headerWrite.nameAllowsNewline() and
+        this = headerWrite.getNameArg()
+        or
+        headerWrite.valueAllowsNewline() and
+        this = headerWrite.getValueArg()
       )
     }
   }

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

@@ -5,7 +5,6 @@
 private import experimental.semmle.python.frameworks.Stdlib
 private import experimental.semmle.python.frameworks.Flask
 private import experimental.semmle.python.frameworks.Django
-private import experimental.semmle.python.frameworks.Werkzeug
 private import experimental.semmle.python.frameworks.LDAP
 private import experimental.semmle.python.frameworks.JWT
 private import experimental.semmle.python.frameworks.Csv

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

@@ -88,31 +88,6 @@ private module ExperimentalPrivateDjango {
             result = baseClassRef().getReturn().getAMember()
           }
 
-          class DjangoResponseSetItemCall extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
-            DjangoResponseSetItemCall() {
-              this = baseClassRef().getReturn().getMember("__setitem__").getACall()
-            }
-
-            override DataFlow::Node getNameArg() { result = this.getArg(0) }
-
-            override DataFlow::Node getValueArg() { result = this.getArg(1) }
-          }
-
-          class DjangoResponseDefinition extends DataFlow::Node, HeaderDeclaration::Range {
-            DataFlow::Node headerInput;
-
-            DjangoResponseDefinition() {
-              headerInput = headerInstance().asSink() and
-              headerInput.asCfgNode() = this.asCfgNode().(DefinitionNode).getValue()
-            }
-
-            override DataFlow::Node getNameArg() {
-              result.asExpr() = this.asExpr().(Subscript).getIndex()
-            }
-
-            override DataFlow::Node getValueArg() { result = headerInput }
-          }
-
           /**
            * Gets a call to `set_cookie()`.
            *

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

@@ -42,32 +42,6 @@ module ExperimentalFlask {
     headerInstance().asExpr() = result.asExpr().(Subscript).getObject()
   }
 
-  class FlaskHeaderDefinition extends DataFlow::Node, HeaderDeclaration::Range {
-    DataFlow::Node headerInput;
-
-    FlaskHeaderDefinition() {
-      this.asCfgNode().(DefinitionNode) = headerInstanceCall().asCfgNode() and
-      headerInput.asCfgNode() = this.asCfgNode().(DefinitionNode).getValue()
-    }
-
-    override DataFlow::Node getNameArg() { result.asExpr() = this.asExpr().(Subscript).getIndex() }
-
-    override DataFlow::Node getValueArg() { result = headerInput }
-  }
-
-  private class FlaskMakeResponseExtend extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
-    KeyValuePair item;
-
-    FlaskMakeResponseExtend() {
-      this.getFunction() = headerInstanceCall() and
-      item = this.getArg(_).asExpr().(Dict).getAnItem()
-    }
-
-    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;