Merge pull request #16856 from owen-mc/go/mad-sources-chi-echo-elazarl

Go: Convert chi echo and elazarl sources to MaD
github · Jul 16, 2024 · 2dc63ef · 2dc63ef
2 parents 18cde3b + 3efbee0
commit 2dc63ef
Show file tree

Hide file tree

Showing 46 changed files with 290 additions and 332 deletions.
diff --git a/go/ql/lib/ext/github.com.elazarl.goproxy.model.yml b/go/ql/lib/ext/github.com.elazarl.goproxy.model.yml
@@ -5,3 +5,10 @@ extensions:
     data:
       - ["github.com/elazarl/goproxy", "CertStorage", True, "Fetch", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
       - ["github.com/elazarl/goproxy", "CertStorage", True, "Fetch", "", "", "Argument[1]", "ReturnValue[0]", "taint", "manual"]
+
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sourceModel
+    data:
+      - ["github.com/elazarl/goproxy", "ProxyCtx", True, "UserData", "", "", "", "remote", "manual"]
+      - ["github.com/elazarl/goproxy", "ProxyCtx", True, "Charset", "", "", "ReturnValue", "remote", "manual"]
diff --git a/go/ql/lib/ext/github.com.go-chi.chi.model.yml b/go/ql/lib/ext/github.com.go-chi.chi.model.yml
@@ -0,0 +1,8 @@
+extensions:
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sourceModel
+    data:
+      - ["github.com/go-chi/chi", "", True, "URLParam", "", "", "ReturnValue", "remote", "manual"]
+      - ["github.com/go-chi/chi", "", True, "URLParamFromCtx", "", "", "ReturnValue", "remote", "manual"]
+      - ["github.com/go-chi/chi", "Context", True, "URLParam", "", "", "ReturnValue", "remote", "manual"]
diff --git a/go/ql/lib/ext/github.com.labstack.echo.model.yml b/go/ql/lib/ext/github.com.labstack.echo.model.yml
@@ -5,3 +5,20 @@ extensions:
     data:
       - ["github.com/labstack/echo", "Context", True, "Get", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
       - ["github.com/labstack/echo", "Context", True, "Set", "", "", "Argument[1]", "Argument[receiver]", "taint", "manual"]
+
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sourceModel
+    data:
+      - ["github.com/labstack/echo", "Context", True, "Bind", "", "", "Argument[0]", "remote", "manual"]
+      - ["github.com/labstack/echo", "Context", True, "Param", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["github.com/labstack/echo", "Context", True, "ParamValues", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["github.com/labstack/echo", "Context", True, "QueryParam", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["github.com/labstack/echo", "Context", True, "QueryParams", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["github.com/labstack/echo", "Context", True, "QueryString", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["github.com/labstack/echo", "Context", True, "FormValue", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["github.com/labstack/echo", "Context", True, "FormParams", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["github.com/labstack/echo", "Context", True, "FormFile", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["github.com/labstack/echo", "Context", True, "MultipartForm", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["github.com/labstack/echo", "Context", True, "Cookie", "", "", "ReturnValue[0]", "remote", "manual"]
+      - ["github.com/labstack/echo", "Context", True, "Cookies", "", "", "ReturnValue[0]", "remote", "manual"]
diff --git a/go/ql/lib/go.qll b/go/ql/lib/go.qll
@@ -34,7 +34,6 @@ import semmle.go.frameworks.Afero
 import semmle.go.frameworks.AwsLambda
 import semmle.go.frameworks.Beego
 import semmle.go.frameworks.BeegoOrm
-import semmle.go.frameworks.Chi
 import semmle.go.frameworks.Couchbase
 import semmle.go.frameworks.Echo
 import semmle.go.frameworks.ElazarlGoproxy

diff --git a/go/ql/lib/semmle/go/frameworks/Chi.qll b/go/ql/lib/semmle/go/frameworks/Chi.qll
diff --git a/go/ql/lib/semmle/go/frameworks/Echo.qll b/go/ql/lib/semmle/go/frameworks/Echo.qll
@@ -9,23 +9,6 @@ private module Echo {
   /** Gets the package name `github.com/labstack/echo`. */
   private string packagePath() { result = package("github.com/labstack/echo", "") }
 
-  /**
-   * Data from a `Context` interface method, considered as a source of remote flow.
-   */
-  private class EchoContextSource extends RemoteFlowSource::Range {
-    EchoContextSource() {
-      exists(DataFlow::MethodCallNode call, string methodName |
-        methodName =
-          [
-            "Param", "ParamValues", "QueryParam", "QueryParams", "QueryString", "FormValue",
-            "FormParams", "FormFile", "MultipartForm", "Cookie", "Cookies"
-          ] and
-        call.getTarget().hasQualifiedName(packagePath(), "Context", methodName) and
-        this = call.getResult(0)
-      )
-    }
-  }
-
   /**
    * Data from a `Context` interface method that is not generally exploitable for open-redirect attacks.
    */
@@ -39,19 +22,6 @@ private module Echo {
     }
   }
 
-  /**
-   * A call to a method on `Context` struct that unmarshals data into a target.
-   */
-  private class EchoContextBinder extends RemoteFlowSource::Range {
-    EchoContextBinder() {
-      exists(DataFlow::MethodCallNode call |
-        call.getTarget().hasQualifiedName(packagePath(), "Context", "Bind")
-      |
-        this = FunctionOutput::parameter(0).getExitNode(call)
-      )
-    }
-  }
-
   /**
    * `echo.Context` methods which set the content-type to `text/html` and write a result in one operation.
    */

diff --git a/go/ql/lib/semmle/go/frameworks/ElazarlGoproxy.qll b/go/ql/lib/semmle/go/frameworks/ElazarlGoproxy.qll
@@ -95,19 +95,6 @@ module ElazarlGoproxy {
     }
   }
 
-  private class UserControlledRequestData extends RemoteFlowSource::Range {
-    UserControlledRequestData() {
-      exists(DataFlow::FieldReadNode frn | this = frn |
-        // liberally consider ProxyCtx.UserData to be untrusted; it's a data field set by a request handler
-        frn.getField().hasQualifiedName(packagePath(), "ProxyCtx", "UserData")
-      )
-      or
-      exists(DataFlow::MethodCallNode call | this = call |
-        call.getTarget().hasQualifiedName(packagePath(), "ProxyCtx", "Charset")
-      )
-    }
-  }
-
   private class ProxyLogFunction extends StringOps::Formatting::Range, Method {
     ProxyLogFunction() { this.hasQualifiedName(packagePath(), "ProxyCtx", ["Logf", "Warnf"]) }
 

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:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:61:3:61:51 | ...+... | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:3:62:33 | slice literal | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:24:62:32 | untrusted | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:66:3:66:11 | untrusted | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:68:3:68:51 | ...+... | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:3:69:33 | slice literal | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:24:69:32 | untrusted | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:73:3:73:11 | untrusted | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:75:3:75:51 | ...+... | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:3:76:33 | slice literal | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:24:76:32 | untrusted | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:80:22:80:30 | untrusted | provenance | Src:MaD:686  |
-| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:81:25:81:33 | untrusted | provenance | Src:MaD:686  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:59:3:59:11 | untrusted | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:61:3:61:51 | ...+... | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:3:62:33 | slice literal | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:24:62:32 | untrusted | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:66:3:66:11 | untrusted | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:68:3:68:51 | ...+... | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:3:69:33 | slice literal | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:24:69:32 | untrusted | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:73:3:73:11 | untrusted | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:75:3:75:51 | ...+... | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:3:76:33 | slice literal | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:24:76:32 | untrusted | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:80:22:80:30 | untrusted | provenance | Src:MaD:703  |
+| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:81:25:81:33 | untrusted | provenance | Src:MaD:703  |
 | 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:667 |
+| timing.go:15:18:15:27 | selection of Header | timing.go:15:18:15:45 | call to Get | provenance | MaD:684 |
 | 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:667 |
+| timing.go:28:18:28:27 | selection of Header | timing.go:28:18:28:45 | call to Get | provenance | MaD:684 |
 | 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:667 |
+| timing.go:41:18:41:27 | selection of Header | timing.go:41:18:41:45 | call to Get | provenance | MaD:684 |
 | 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:747 |
+| ImproperLdapAuth.go:18:18:18:24 | selection of URL | ImproperLdapAuth.go:18:18:18:32 | call to Query | provenance | MaD:764 |
 | 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:747 |
+| DivideByZero.go:10:12:10:16 | selection of URL | DivideByZero.go:10:12:10:24 | call to Query | provenance | MaD:764 |
 | 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:747 |
+| DivideByZero.go:17:12:17:16 | selection of URL | DivideByZero.go:17:12:17:24 | call to Query | provenance | MaD:764 |
 | 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:747 |
+| DivideByZero.go:24:12:24:16 | selection of URL | DivideByZero.go:24:12:24:24 | call to Query | provenance | MaD:764 |
 | 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:747 |
+| DivideByZero.go:31:12:31:16 | selection of URL | DivideByZero.go:31:12:31:24 | call to Query | provenance | MaD:764 |
 | 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:747 |
+| DivideByZero.go:38:12:38:16 | selection of URL | DivideByZero.go:38:12:38:24 | call to Query | provenance | MaD:764 |
 | 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:747 |
+| DivideByZero.go:54:12:54:16 | selection of URL | DivideByZero.go:54:12:54:24 | call to Query | provenance | MaD:764 |
 | 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

diff --git a/go/ql/test/experimental/CWE-522-DecompressionBombs/DecompressionBombs.expected b/go/ql/test/experimental/CWE-522-DecompressionBombs/DecompressionBombs.expected
@@ -1,5 +1,5 @@
 edges
-| test.go:59:16:59:44 | call to FormValue | test.go:128:20:128:27 | definition of filename | provenance | Src:MaD:682  |
+| test.go:59:16:59:44 | call to FormValue | test.go:128:20:128:27 | definition of filename | provenance | Src:MaD:699  |
 | test.go:60:15:60:26 | selection of Body | test.go:158:19:158:22 | definition of file | provenance |  |
 | test.go:61:24:61:35 | selection of Body | test.go:169:28:169:31 | definition of file | provenance |  |
 | test.go:62:13:62:24 | selection of Body | test.go:181:17:181:20 | definition of file | provenance |  |
@@ -31,15 +31,15 @@ edges
 | test.go:145:12:145:19 | call to Open | test.go:147:37:147:38 | rc | provenance |  |
 | test.go:158:19:158:22 | definition of file | test.go:159:25:159:28 | file | provenance |  |
 | test.go:159:2:159:29 | ... := ...[0] | test.go:160:48:160:52 | file1 | provenance |  |
-| test.go:159:25:159:28 | file | test.go:159:2:159:29 | ... := ...[0] | provenance | MaD:559 |
+| test.go:159:25:159:28 | file | test.go:159:2:159:29 | ... := ...[0] | provenance | MaD:576 |
 | test.go:160:2:160:69 | ... := ...[0] | test.go:163:26:163:29 | file | provenance |  |
 | test.go:160:32:160:53 | call to NewReader | test.go:160:2:160:69 | ... := ...[0] | provenance | Config |
 | test.go:160:48:160:52 | file1 | test.go:160:32:160:53 | call to NewReader | provenance | MaD:46 |
 | test.go:163:3:163:36 | ... := ...[0] | test.go:164:36:164:51 | fileReaderCloser | provenance |  |
 | test.go:163:26:163:29 | file | test.go:163:3:163:36 | ... := ...[0] | provenance | MaD:8 |
 | test.go:169:28:169:31 | definition of file | test.go:170:25:170:28 | file | provenance |  |
 | test.go:170:2:170:29 | ... := ...[0] | test.go:171:57:171:61 | file2 | provenance |  |
-| test.go:170:25:170:28 | file | test.go:170:2:170:29 | ... := ...[0] | provenance | MaD:559 |
+| test.go:170:25:170:28 | file | test.go:170:2:170:29 | ... := ...[0] | provenance | MaD:576 |
 | test.go:171:2:171:78 | ... := ...[0] | test.go:175:26:175:29 | file | provenance |  |
 | test.go:171:41:171:62 | call to NewReader | test.go:171:2:171:78 | ... := ...[0] | provenance | Config |
 | test.go:171:57:171:61 | file2 | test.go:171:41:171:62 | call to NewReader | provenance | MaD:46 |

diff --git a/go/ql/test/experimental/CWE-74/DsnInjection.expected b/go/ql/test/experimental/CWE-74/DsnInjection.expected
@@ -1,5 +1,5 @@
 edges
-| Dsn.go:47:10:47:30 | call to FormValue | Dsn.go:49:102:49:105 | name | provenance | Src:MaD:682  |
+| Dsn.go:47:10:47:30 | call to FormValue | Dsn.go:49:102:49:105 | name | provenance | Src:MaD:699  |
 | Dsn.go:49:11:49:106 | []type{args} [array] | Dsn.go:49:11:49:106 | call to Sprintf | provenance | MaD:248 |
 | Dsn.go:49:11:49:106 | call to Sprintf | Dsn.go:50:29:50:33 | dbDSN | provenance |  |
 | Dsn.go:49:102:49:105 | name | Dsn.go:49:11:49:106 | []type{args} [array] | provenance |  |