Merge pull request #16986 from owen-mc/go/mad-sources-revel-nethttp

Go: Convert Revel and net/http sources to MaD
github · Jul 16, 2024 · 12fe998 · 12fe998
2 parents 39f0288 + fc6b17a
commit 12fe998
Show file tree

Hide file tree

Showing 44 changed files with 325 additions and 349 deletions.
diff --git a/go/ql/lib/ext/github.com.revel.revel.model.yml b/go/ql/lib/ext/github.com.revel.revel.model.yml
@@ -1,25 +1,46 @@
 extensions:
+  - addsTo:
+      pack: codeql/go-all
+      extensible: packageGrouping
+    data:
+      - ["revel", "github.com/revel/revel"]
+      - ["revel", "github.com/robfig/revel"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sourceModel
+    data:
+      - ["group:revel", "Controller", True, "Params", "", "", "", "remote", "manual"]
+      - ["group:revel", "Request", True, "Header", "", "", "", "remote", "manual"]
+      - ["group:revel", "Request", True, "ContentType", "", "", "", "remote", "manual"]
+      - ["group:revel", "Request", True, "AcceptLanguages", "", "", "", "remote", "manual"]
+      - ["group:revel", "Request", True, "Locale", "", "", "", "remote", "manual"]
+      - ["group:revel", "Request", True, "URL", "", "", "", "remote", "manual"]
+      - ["group:revel", "Request", True, "Form", "", "", "", "remote", "manual"]
+      - ["group:revel", "Request", True, "MultipartForm", "", "", "", "remote", "manual"]
+      - ["group:revel", "RouteMatch", True, "Params", "", "", "", "remote", "manual"]
+      - ["group:revel", "Request", True, "Cookie", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["group:revel", "Request", True, "FormValue", "", "", "ReturnValue", "remote", "manual"]
+      - ["group:revel", "Request", True, "GetBody", "", "", "ReturnValue", "remote", "manual"]
+      - ["group:revel", "Request", True, "GetForm", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["group:revel", "Request", True, "GetHttpHeader", "", "", "ReturnValue", "remote", "manual"]
+      - ["group:revel", "Request", True, "GetMultipartForm", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["group:revel", "Request", True, "GetQuery", "", "", "ReturnValue", "remote", "manual"]
+      - ["group:revel", "Request", True, "GetRequestURI", "", "", "ReturnValue", "remote", "manual"]
+      - ["group:revel", "Request", True, "MultipartReader", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["group:revel", "Request", True, "PostFormValue", "", "", "ReturnValue", "remote", "manual"]
+      - ["group:revel", "Request", True, "Referer", "", "", "ReturnValue", "remote", "manual"]
+      - ["group:revel", "Request", True, "UserAgent", "", "", "ReturnValue", "remote", "manual"]
   - addsTo:
       pack: codeql/go-all
       extensible: summaryModel
     data:
-      - ["github.com/revel/revel", "Params", True, "Bind", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
-      - ["github.com/revel/revel", "Params", True, "BindJSON", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
-      - ["github.com/revel/revel", "RevelHeader", True, "Add", "", "", "Argument[0..1]", "Argument[receiver]", "taint", "manual"]
-      - ["github.com/revel/revel", "RevelHeader", True, "Get", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
-      - ["github.com/revel/revel", "RevelHeader", True, "GetAll", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
-      - ["github.com/revel/revel", "RevelHeader", True, "Set", "", "", "Argument[0..1]", "Argument[receiver]", "taint", "manual"]
-      - ["github.com/revel/revel", "RevelHeader", True, "SetCookie", "", "", "Argument[0]", "Argument[receiver]", "taint", "manual"]
-      - ["github.com/revel/revel", "ServerCookie", True, "GetValue", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
-      - ["github.com/revel/revel", "ServerMultipartForm", True, "GetFiles", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
-      - ["github.com/revel/revel", "ServerMultipartForm", True, "GetValues", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
-      - ["github.com/robfig/revel", "Params", True, "Bind", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
-      - ["github.com/robfig/revel", "Params", True, "BindJSON", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
-      - ["github.com/robfig/revel", "RevelHeader", True, "Add", "", "", "Argument[0..1]", "Argument[receiver]", "taint", "manual"]
-      - ["github.com/robfig/revel", "RevelHeader", True, "Get", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
-      - ["github.com/robfig/revel", "RevelHeader", True, "GetAll", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
-      - ["github.com/robfig/revel", "RevelHeader", True, "Set", "", "", "Argument[0..1]", "Argument[receiver]", "taint", "manual"]
-      - ["github.com/robfig/revel", "RevelHeader", True, "SetCookie", "", "", "Argument[0]", "Argument[receiver]", "taint", "manual"]
-      - ["github.com/robfig/revel", "ServerCookie", True, "GetValue", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
-      - ["github.com/robfig/revel", "ServerMultipartForm", True, "GetFiles", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
-      - ["github.com/robfig/revel", "ServerMultipartForm", True, "GetValues", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
+      - ["group:revel", "Params", True, "Bind", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
+      - ["group:revel", "Params", True, "BindJSON", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
+      - ["group:revel", "RevelHeader", True, "Add", "", "", "Argument[0..1]", "Argument[receiver]", "taint", "manual"]
+      - ["group:revel", "RevelHeader", True, "Get", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
+      - ["group:revel", "RevelHeader", True, "GetAll", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
+      - ["group:revel", "RevelHeader", True, "Set", "", "", "Argument[0..1]", "Argument[receiver]", "taint", "manual"]
+      - ["group:revel", "RevelHeader", True, "SetCookie", "", "", "Argument[0]", "Argument[receiver]", "taint", "manual"]
+      - ["group:revel", "ServerCookie", True, "GetValue", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
+      - ["group:revel", "ServerMultipartForm", True, "GetFiles", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
+      - ["group:revel", "ServerMultipartForm", True, "GetValues", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
diff --git a/go/ql/lib/ext/net.http.model.yml b/go/ql/lib/ext/net.http.model.yml
@@ -37,3 +37,11 @@ extensions:
       - ["net/http", "Request", True, "PostFormValue", "", "", "ReturnValue", "remote", "manual"]
       - ["net/http", "Request", True, "Referer", "", "", "ReturnValue", "remote", "manual"]
       - ["net/http", "Request", True, "UserAgent", "", "", "ReturnValue", "remote", "manual"]
+      - ["net/http", "Request", True, "Body", "", "", "", "remote", "manual"]
+      - ["net/http", "Request", True, "GetBody", "", "", "", "remote", "manual"]
+      - ["net/http", "Request", True, "Form", "", "", "", "remote", "manual"]
+      - ["net/http", "Request", True, "PostForm", "", "", "", "remote", "manual"]
+      - ["net/http", "Request", True, "MultipartForm", "", "", "", "remote", "manual"]
+      - ["net/http", "Request", True, "Header", "", "", "", "remote", "manual"]
+      - ["net/http", "Request", True, "Trailer", "", "", "", "remote", "manual"]
+      - ["net/http", "Request", True, "URL", "", "", "", "remote", "manual"]
diff --git a/go/ql/lib/semmle/go/frameworks/Revel.qll b/go/ql/lib/semmle/go/frameworks/Revel.qll
@@ -12,15 +12,6 @@ module Revel {
     result = package(["github.com/revel", "github.com/robfig"] + "/revel", "")
   }
 
-  private class ControllerParams extends RemoteFlowSource::Range, DataFlow::FieldReadNode {
-    ControllerParams() {
-      exists(Field f |
-        this.readsField(_, f) and
-        f.hasQualifiedName(packagePath(), "Controller", "Params")
-      )
-    }
-  }
-
   private class ParamsFixedSanitizer extends TaintTracking::DefaultTaintSanitizer,
     DataFlow::FieldReadNode
   {
@@ -32,41 +23,6 @@ module Revel {
     }
   }
 
-  private class RouteMatchParams extends RemoteFlowSource::Range, DataFlow::FieldReadNode {
-    RouteMatchParams() {
-      exists(Field f |
-        this.readsField(_, f) and
-        f.hasQualifiedName(packagePath(), "RouteMatch", "Params")
-      )
-    }
-  }
-
-  /** An access to an HTTP request field whose value may be controlled by an untrusted user. */
-  private class UserControlledRequestField extends RemoteFlowSource::Range, DataFlow::FieldReadNode {
-    UserControlledRequestField() {
-      exists(string fieldName |
-        this.getField().hasQualifiedName(packagePath(), "Request", fieldName)
-      |
-        fieldName in [
-            "Header", "ContentType", "AcceptLanguages", "Locale", "URL", "Form", "MultipartForm"
-          ]
-      )
-    }
-  }
-
-  private class UserControlledRequestMethod extends RemoteFlowSource::Range,
-    DataFlow::MethodCallNode
-  {
-    UserControlledRequestMethod() {
-      this.getTarget()
-          .hasQualifiedName(packagePath(), "Request",
-            [
-              "FormValue", "PostFormValue", "GetQuery", "GetForm", "GetMultipartForm", "GetBody",
-              "Cookie", "GetHttpHeader", "GetRequestURI", "MultipartReader", "Referer", "UserAgent"
-            ])
-    }
-  }
-
   private string contentTypeFromFilename(DataFlow::Node filename) {
     if filename.getStringValue().regexpMatch("(?i).*\\.html?")
     then result = "text/html"

diff --git a/go/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll b/go/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll
@@ -8,16 +8,6 @@ private import semmle.go.dataflow.internal.FlowSummaryImpl::Private
 
 /** Provides models of commonly used functions in the `net/http` package. */
 module NetHttp {
-  /** An access to an HTTP request field whose value may be controlled by an untrusted user. */
-  private class UserControlledRequestField extends RemoteFlowSource::Range, DataFlow::FieldReadNode {
-    UserControlledRequestField() {
-      exists(string fieldName | this.getField().hasQualifiedName("net/http", "Request", fieldName) |
-        fieldName =
-          ["Body", "GetBody", "Form", "PostForm", "MultipartForm", "Header", "Trailer", "URL"]
-      )
-    }
-  }
-
   /** The declaration of a variable which either is or has a field that implements the http.ResponseWriter type */
   private class StdlibResponseWriter extends Http::ResponseWriter::Range {
     SsaWithFields v;

diff --git a/go/ql/test/experimental/CWE-090/LDAPInjection.expected b/go/ql/test/experimental/CWE-090/LDAPInjection.expected
@@ -1,18 +1,18 @@
 edges
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:59:3:59:11 | untrusted | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:61:3:61:51 | ...+... | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:3:62:33 | slice literal | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:24:62:32 | untrusted | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:66:3:66:11 | untrusted | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:68:3:68:51 | ...+... | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:3:69:33 | slice literal | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:24:69:32 | untrusted | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:73:3:73:11 | untrusted | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:75:3:75:51 | ...+... | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:3:76:33 | slice literal | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:24:76:32 | untrusted | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:80:22:80:30 | untrusted | provenance | Src:MaD:757  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:81:25:81:33 | untrusted | provenance | Src:MaD:757  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:59:3:59:11 | untrusted | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:61:3:61:51 | ...+... | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:3:62:33 | slice literal | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:24:62:32 | untrusted | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:66:3:66:11 | untrusted | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:68:3:68:51 | ...+... | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:3:69:33 | slice literal | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:24:69:32 | untrusted | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:73:3:73:11 | untrusted | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:75:3:75:51 | ...+... | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:3:76:33 | slice literal | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:24:76:32 | untrusted | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:80:22:80:30 | untrusted | provenance | Src:MaD:770  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:81:25:81:33 | untrusted | provenance | Src:MaD:770  |
 | LDAPInjection.go:62:3:62:33 | slice literal [array] | LDAPInjection.go:62:3:62:33 | slice literal | provenance |  |
 | LDAPInjection.go:62:24:62:32 | untrusted | LDAPInjection.go:62:3:62:33 | slice literal [array] | provenance |  |
 | LDAPInjection.go:69:3:69:33 | slice literal [array] | LDAPInjection.go:69:3:69:33 | slice literal | provenance |  |

diff --git a/go/ql/test/experimental/CWE-203/Timing.expected b/go/ql/test/experimental/CWE-203/Timing.expected
@@ -1,9 +1,9 @@
 edges
-| timing.go:15:18:15:27 | selection of Header | timing.go:15:18:15:45 | call to Get | provenance | MaD:738 |
+| timing.go:15:18:15:27 | selection of Header | timing.go:15:18:15:45 | call to Get | provenance | Src:MaD:776 MaD:751 |
 | timing.go:15:18:15:45 | call to Get | timing.go:17:31:17:42 | headerSecret | provenance |  |
-| timing.go:28:18:28:27 | selection of Header | timing.go:28:18:28:45 | call to Get | provenance | MaD:738 |
+| timing.go:28:18:28:27 | selection of Header | timing.go:28:18:28:45 | call to Get | provenance | Src:MaD:776 MaD:751 |
 | timing.go:28:18:28:45 | call to Get | timing.go:30:47:30:58 | headerSecret | provenance |  |
-| timing.go:41:18:41:27 | selection of Header | timing.go:41:18:41:45 | call to Get | provenance | MaD:738 |
+| timing.go:41:18:41:27 | selection of Header | timing.go:41:18:41:45 | call to Get | provenance | Src:MaD:776 MaD:751 |
 | timing.go:41:18:41:45 | call to Get | timing.go:42:25:42:36 | headerSecret | provenance |  |
 nodes
 | timing.go:15:18:15:27 | selection of Header | semmle.label | selection of Header |

diff --git a/go/ql/test/experimental/CWE-287/ImproperLdapAuth.expected b/go/ql/test/experimental/CWE-287/ImproperLdapAuth.expected
@@ -1,5 +1,5 @@
 edges
-| ImproperLdapAuth.go:18:18:18:24 | selection of URL | ImproperLdapAuth.go:18:18:18:32 | call to Query | provenance | MaD:818 |
+| ImproperLdapAuth.go:18:18:18:24 | selection of URL | ImproperLdapAuth.go:18:18:18:32 | call to Query | provenance | Src:MaD:778 MaD:839 |
 | ImproperLdapAuth.go:18:18:18:32 | call to Query | ImproperLdapAuth.go:28:23:28:34 | bindPassword | provenance |  |
 | ImproperLdapAuth.go:87:18:87:19 | "" | ImproperLdapAuth.go:97:23:97:34 | bindPassword | provenance |  |
 nodes

diff --git a/go/ql/test/experimental/CWE-369/DivideByZero.expected b/go/ql/test/experimental/CWE-369/DivideByZero.expected
@@ -1,24 +1,24 @@
 edges
-| DivideByZero.go:10:12:10:16 | selection of URL | DivideByZero.go:10:12:10:24 | call to Query | provenance | MaD:818 |
+| DivideByZero.go:10:12:10:16 | selection of URL | DivideByZero.go:10:12:10:24 | call to Query | provenance | Src:MaD:778 MaD:839 |
 | DivideByZero.go:10:12:10:24 | call to Query | DivideByZero.go:11:27:11:32 | param1 | provenance |  |
 | DivideByZero.go:11:2:11:33 | ... := ...[0] | DivideByZero.go:12:16:12:20 | value | provenance |  |
 | DivideByZero.go:11:27:11:32 | param1 | DivideByZero.go:11:2:11:33 | ... := ...[0] | provenance | Config |
-| DivideByZero.go:17:12:17:16 | selection of URL | DivideByZero.go:17:12:17:24 | call to Query | provenance | MaD:818 |
+| DivideByZero.go:17:12:17:16 | selection of URL | DivideByZero.go:17:12:17:24 | call to Query | provenance | Src:MaD:778 MaD:839 |
 | DivideByZero.go:17:12:17:24 | call to Query | DivideByZero.go:18:11:18:24 | type conversion | provenance |  |
 | DivideByZero.go:18:11:18:24 | type conversion | DivideByZero.go:19:16:19:20 | value | provenance |  |
-| DivideByZero.go:24:12:24:16 | selection of URL | DivideByZero.go:24:12:24:24 | call to Query | provenance | MaD:818 |
+| DivideByZero.go:24:12:24:16 | selection of URL | DivideByZero.go:24:12:24:24 | call to Query | provenance | Src:MaD:778 MaD:839 |
 | DivideByZero.go:24:12:24:24 | call to Query | DivideByZero.go:25:31:25:36 | param1 | provenance |  |
 | DivideByZero.go:25:2:25:45 | ... := ...[0] | DivideByZero.go:26:16:26:20 | value | provenance |  |
 | DivideByZero.go:25:31:25:36 | param1 | DivideByZero.go:25:2:25:45 | ... := ...[0] | provenance | Config |
-| DivideByZero.go:31:12:31:16 | selection of URL | DivideByZero.go:31:12:31:24 | call to Query | provenance | MaD:818 |
+| DivideByZero.go:31:12:31:16 | selection of URL | DivideByZero.go:31:12:31:24 | call to Query | provenance | Src:MaD:778 MaD:839 |
 | DivideByZero.go:31:12:31:24 | call to Query | DivideByZero.go:32:33:32:38 | param1 | provenance |  |
 | DivideByZero.go:32:2:32:43 | ... := ...[0] | DivideByZero.go:33:16:33:20 | value | provenance |  |
 | DivideByZero.go:32:33:32:38 | param1 | DivideByZero.go:32:2:32:43 | ... := ...[0] | provenance | Config |
-| DivideByZero.go:38:12:38:16 | selection of URL | DivideByZero.go:38:12:38:24 | call to Query | provenance | MaD:818 |
+| DivideByZero.go:38:12:38:16 | selection of URL | DivideByZero.go:38:12:38:24 | call to Query | provenance | Src:MaD:778 MaD:839 |
 | DivideByZero.go:38:12:38:24 | call to Query | DivideByZero.go:39:32:39:37 | param1 | provenance |  |
 | DivideByZero.go:39:2:39:46 | ... := ...[0] | DivideByZero.go:40:16:40:20 | value | provenance |  |
 | DivideByZero.go:39:32:39:37 | param1 | DivideByZero.go:39:2:39:46 | ... := ...[0] | provenance | Config |
-| DivideByZero.go:54:12:54:16 | selection of URL | DivideByZero.go:54:12:54:24 | call to Query | provenance | MaD:818 |
+| DivideByZero.go:54:12:54:16 | selection of URL | DivideByZero.go:54:12:54:24 | call to Query | provenance | Src:MaD:778 MaD:839 |
 | DivideByZero.go:54:12:54:24 | call to Query | DivideByZero.go:55:11:55:24 | type conversion | provenance |  |
 | DivideByZero.go:55:11:55:24 | type conversion | DivideByZero.go:57:17:57:21 | value | provenance |  |
 nodes