Add test case for validated wsgiref servers + fix typo

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

@@ -2183,15 +2183,24 @@ module StdlibPrivate {
      * for how a request is processed and given to an application.
      */
     class WsgirefSimpleServerApplication extends Http::Server::RequestHandler::Range {
+      boolean validator;
+
       WsgirefSimpleServerApplication() {
         exists(DataFlow::Node appArg, DataFlow::CallCfgNode setAppCall |
           (
             setAppCall =
-              WsgirefSimpleServer::subclassRef().getReturn().getMember("set_app").getACall()
+              WsgirefSimpleServer::subclassRef().getReturn().getMember("set_app").getACall() and
+            validator = false
             or
             setAppCall
                 .(DataFlow::MethodCallNode)
-                .calls(any(WsgiServerSubclass cls).getASelfRef(), "set_app")
+                .calls(any(WsgiServerSubclass cls).getASelfRef(), "set_app") and
+            validator = false
+            or
+            // assume an application that is passed to `wsgiref.validate.validator` is eventually passed to `set_app`
+            setAppCall =
+              API::moduleImport("wsgiref").getMember("validate").getMember("validator").getACall() and
+            validator = true
           ) and
           appArg in [setAppCall.getArg(0), setAppCall.getArgByName("application")]
           or
@@ -2201,7 +2210,8 @@ module StdlibPrivate {
                 .getMember("simple_server")
                 .getMember("make_server")
                 .getACall() and
-          appArg in [setAppCall.getArg(2), setAppCall.getArgByName("app")]
+          appArg in [setAppCall.getArg(2), setAppCall.getArgByName("app")] and
+          validator = false
         |
           appArg = poorMansFunctionTracker(this)
         )
@@ -2210,6 +2220,9 @@ module StdlibPrivate {
       override Parameter getARoutedParameter() { none() }
 
       override string getFramework() { result = "Stdlib: wsgiref.simple_server application" }
+
+      /** Holds if this simple server application was passed to `wsgiref.validate.validator`. */
+      predicate isValidated() { validator = true }
     }
 
     /**
@@ -2324,7 +2337,7 @@ module StdlibPrivate {
       API::Node classRef() {
         result = API::moduleImport("wsgiref").getMember("headers").getMember("Headers")
         or
-        result = ModelOutput::getATypeNode("wsqiref.headers.Headers~Subclass").getASubclass*()
+        result = ModelOutput::getATypeNode("wsgiref.headers.Headers~Subclass").getASubclass*()
       }
 
       /** Gets a reference to an instance of `wsgiref.headers.Headers`. */
@@ -2338,6 +2351,11 @@ module StdlibPrivate {
       /** Gets a reference to an instance of `wsgiref.headers.Headers`. */
       DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
 
+      /** Holds if there exists an application that is validated by `wsgiref.validate.validator`. */
+      private predicate existsValidatedApplication() {
+        exists(WsgirefSimpleServerApplication app | app.isValidated())
+      }
+
       /** A class instantiation of `wsgiref.headers.Headers`, conidered as a write to a response header. */
       private class WsgirefHeadersInstantiation extends Http::Server::ResponseHeaderBulkWrite::Range,
         DataFlow::CallCfgNode
@@ -2348,28 +2366,10 @@ module StdlibPrivate {
           result = [this.getArg(0), this.getArgByName("headers")]
         }
 
-        // TODO: implement validator
-        override predicate nameAllowsNewline() { any() }
-
-        override predicate valueAllowsNewline() { any() }
-      }
-
-      /**
-       * A call to a `start_response` function that sets the response headers.
-       */
-      private class WsgirefSimpleServerSetHeaders extends Http::Server::ResponseHeaderBulkWrite::Range,
-        DataFlow::CallCfgNode
-      {
-        WsgirefSimpleServerSetHeaders() { this.getFunction() = startResponse() }
-
-        override DataFlow::Node getBulkArg() {
-          result = [this.getArg(1), this.getArgByName("headers")]
-        }
-
-        // TODO: implement validator
-        override predicate nameAllowsNewline() { any() }
+        // TODO: These checks perhaps could be made more precise.
+        override predicate nameAllowsNewline() { not existsValidatedApplication() }
 
-        override predicate valueAllowsNewline() { any() }
+        override predicate valueAllowsNewline() { not existsValidatedApplication() }
       }
 
       /** A call to a method that writes to a response header. */
@@ -2384,10 +2384,10 @@ module StdlibPrivate {
 
         override DataFlow::Node getValueArg() { result = this.getArg(1) }
 
-        // TODO: implement validator
-        override predicate nameAllowsNewline() { any() }
+        // TODO: These checks perhaps could be made more precise.
+        override predicate nameAllowsNewline() { not existsValidatedApplication() }
 
-        override predicate valueAllowsNewline() { any() }
+        override predicate valueAllowsNewline() { not existsValidatedApplication() }
       }
 
       /** A dict-like write to a response header. */
@@ -2410,10 +2410,28 @@ module StdlibPrivate {
 
         override DataFlow::Node getValueArg() { result = value }
 
-        // TODO: implement validator
-        override predicate nameAllowsNewline() { any() }
+        // TODO: These checks perhaps could be made more precise.
+        override predicate nameAllowsNewline() { not existsValidatedApplication() }
+
+        override predicate valueAllowsNewline() { not existsValidatedApplication() }
+      }
+
+      /**
+       * A call to a `start_response` function that sets the response headers.
+       */
+      private class WsgirefSimpleServerSetHeaders extends Http::Server::ResponseHeaderBulkWrite::Range,
+        DataFlow::CallCfgNode
+      {
+        WsgirefSimpleServerSetHeaders() { this.getFunction() = startResponse() }
+
+        override DataFlow::Node getBulkArg() {
+          result = [this.getArg(1), this.getArgByName("headers")]
+        }
+
+        // TODO: These checks perhaps could be made more precise.
+        override predicate nameAllowsNewline() { not existsValidatedApplication() }
 
-        override predicate valueAllowsNewline() { any() }
+        override predicate valueAllowsNewline() { not existsValidatedApplication() }
       }
     }
   }

python/ql/test/query-tests/Security/CWE-113-HeaderInjection/Tests2/HeaderInjection.expected

@@ -0,0 +1,4 @@
+edges
+nodes
+subpaths
+#select

python/ql/test/query-tests/Security/CWE-113-HeaderInjection/Tests2/HeaderInjection.qlref

@@ -0,0 +1 @@
+Security/CWE-113/HeaderInjection.ql

python/ql/test/query-tests/Security/CWE-113-HeaderInjection/Tests2/wsgiref_tests.py

@@ -0,0 +1,33 @@
+from wsgiref.simple_server import make_server 
+from wsgiref.headers import Headers
+from wsgiref.validate import validator
+
+def test_app(environ, start_response):
+    status = "200 OK"
+    h_name = environ["source_n"]
+    h_val = environ["source_v"]
+    headers = [(h_name, "val"), ("name", h_val)]
+    start_response(status, headers) # GOOD - the application is validated, so headers containing newlines will be rejected.
+    return [b"Hello"]
+
+def test_app2(environ, start_response):
+    status = "200 OK"
+    h_name = environ["source_n"]
+    h_val = environ["source_v"]
+    headers = Headers([(h_name, "val"), ("name", h_val)]) # GOOD
+    headers.add_header(h_name, h_val) # GOOD
+    headers.setdefault(h_name, h_val) # GOOD
+    headers.__setitem__(h_name, h_val) # GOOD
+    headers[h_name] = h_val # GOOD
+    start_response(status, headers)
+    return [b"Hello"]
+
+def main1():
+    with make_server('', 8000, validate(test_app)) as httpd:
+        print("Serving on port 8000...")
+        httpd.serve_forever()
+
+def main2():
+    with make_server('', 8000, validate(test_app2)) as httpd:
+        print("Serving on port 8000...")
+        httpd.serve_forever()