Generalise model of HTTP libraries

* Allow for HTTP response methods that define a content-type without a corresponding header write
* Factor out stdlib-http-specific classification of fields that aren't vulnerable to an open-redirect exploit

Author: smowton

ql/src/semmle/go/Concepts.qll

@@ -509,6 +509,17 @@ module HTTP {
     abstract class Range extends DataFlow::Node {
       /** Gets the response writer associated with this header write, if any. */
       abstract ResponseWriter getResponseWriter();
+
+      /** Gets a content-type associated with this body. */
+      string getAContentType() { result = getAContentTypeNode().getStringValue() }
+
+      /** Gets a dataflow node for a content-type associated with this body. */
+      DataFlow::Node getAContentTypeNode() {
+        exists(HTTP::HeaderWrite hw | hw = getResponseWriter().getAHeaderWrite() |
+          hw.getName().getStringValue().toLowerCase() = "content-type" and
+          result = hw.getValue()
+        )
+      }
     }
   }
 
@@ -525,6 +536,12 @@ module HTTP {
 
     /** Gets the response writer associated with this header write, if any. */
     ResponseWriter getResponseWriter() { result = self.getResponseWriter() }
+
+    /** Gets a content-type associated with this body. */
+    string getAContentType() { result = self.getAContentType() }
+
+    /** Gets a dataflow node for a content-type associated with this body. */
+    DataFlow::Node getAContentTypeNode() { result = self.getAContentTypeNode() }
   }
 
   /** Provides a class for modeling new HTTP client request APIs. */
@@ -576,7 +593,7 @@ module HTTP {
       /** Gets the data-flow node representing the URL being redirected to. */
       abstract DataFlow::Node getUrl();
 
-      /** Gets the response writer that this redirect is sent on. */
+      /** Gets the response writer that this redirect is sent on, if any. */
       abstract ResponseWriter getResponseWriter();
     }
 
@@ -591,6 +608,12 @@ module HTTP {
 
       override ResponseWriter getResponseWriter() { result = HeaderWrite.super.getResponseWriter() }
     }
+
+    /**
+     * An HTTP request attribute that is generally not attacker-controllable for
+     * open redirect exploits; for example, a form field submitted in a POST request.
+     */
+    abstract class UnexploitableSource extends DataFlow::Node { }
   }
 
   /**
@@ -607,7 +630,7 @@ module HTTP {
     /** Gets the data-flow node representing the URL being redirected to. */
     DataFlow::Node getUrl() { result = self.getUrl() }
 
-    /** Gets the response writer that this redirect is sent on. */
+    /** Gets the response writer that this redirect is sent on, if any. */
     ResponseWriter getResponseWriter() { result = self.getResponseWriter() }
   }
 }

ql/src/semmle/go/frameworks/stdlib/NetHttp.qll

@@ -216,6 +216,25 @@ module NetHttp {
     }
   }
 
+  /** Fields and methods of `net/http.Request` that are not generally exploitable in an open-redirect attack. */
+  private class RedirectUnexploitableRequestFields extends HTTP::Redirect::UnexploitableSource {
+    RedirectUnexploitableRequestFields() {
+      exists(Field f, string fieldName |
+        f.hasQualifiedName("net/http", "Request", fieldName) and
+        this = f.getARead()
+      |
+        fieldName = ["Body", "GetBody", "PostForm", "MultipartForm", "Header", "Trailer"]
+      )
+      or
+      exists(Method m, string methName |
+        m.hasQualifiedName("net/http", "Request", methName) and
+        this = m.getACall()
+      |
+        methName = ["Cookie", "Cookies", "MultipartReader", "PostFormValue", "Referer", "UserAgent"]
+      )
+    }
+  }
+
   private class FunctionModels extends TaintTracking::FunctionModel {
     FunctionInput inp;
     FunctionOutput outp;

ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll

@@ -41,28 +41,7 @@ module OpenUrlRedirect {
     UntrustedFlowAsSource() {
       // exclude some fields and methods of URLs that are generally not attacker-controllable for
       // open redirect exploits
-      not exists(Field f, string fieldName |
-        f.hasQualifiedName("net/http", "Request", fieldName) and
-        this = f.getARead()
-      |
-        fieldName = "Body" or
-        fieldName = "GetBody" or
-        fieldName = "PostForm" or
-        fieldName = "MultipartForm" or
-        fieldName = "Header" or
-        fieldName = "Trailer"
-      ) and
-      not exists(Method m, string methName |
-        m.hasQualifiedName("net/http", "Request", methName) and
-        this = m.getACall()
-      |
-        methName = "Cookie" or
-        methName = "Cookies" or
-        methName = "MultipartReader" or
-        methName = "PostFormValues" or
-        methName = "Referer" or
-        methName = "UserAgent"
-      )
+      not this instanceof HTTP::Redirect::UnexploitableSource
     }
   }
 

ql/src/semmle/go/security/ReflectedXssCustomizations.qll

@@ -47,9 +47,7 @@ module ReflectedXss {
    * Holds if `body` specifies the response's content type to be HTML.
    */
   private predicate htmlTypeSpecified(HTTP::ResponseBody body) {
-    exists(HTTP::HeaderWrite hw, string tp | hw = body.getResponseWriter().getAHeaderWrite() |
-      hw.definesHeader("content-type", tp) and tp.regexpMatch("(?i).*html.*")
-    )
+    body.getAContentType().regexpMatch("(?i).*html.*")
   }
 
   /**
@@ -58,9 +56,7 @@ module ReflectedXss {
   private predicate nonHtmlContentType(HTTP::ResponseBody body) {
     not htmlTypeSpecified(body) and
     (
-      exists(HTTP::HeaderWrite hw | hw = body.getResponseWriter().getAHeaderWrite() |
-        hw.getName().getStringValue().toLowerCase() = "content-type"
-      )
+      exists(body.getAContentTypeNode())
       or
       exists(DataFlow::CallNode call | call.getTarget().hasQualifiedName("fmt", "Fprintf") |
         body = call.getAnArgument() and