From 3aacc850094d07abe3ebdfd08fdff66a5777810c Mon Sep 17 00:00:00 2001
From: Porcupiney Hairs <porcupiney.hairs@protonmail.com>
Date: Sun, 23 Jun 2024 21:28:39 +0530
Subject: [PATCH] WIP: Go: CORS Bypass due to incorrect checks

Most Go frameworks provide a function call where-in you can pass a handler for testing origins and performing CORS checks.
These functions typically check for the supllied origin in a list of valid origins. This behaviour is mostly fine but can lead to issues when done incorrectly. for example, consider the code snippets below

https://github.com/zeromicro/go-zero/blob/5c9fae7e6258fd66d026793e7cb03ba9955e3dee/rest/internal/cors/handlers.go#L79-L91

https://github.com/play-with-docker/play-with-docker/blob/7188d83f867cbc201aef4b0597ac5f868c1971f3/handlers/bootstrap.go#L71-L80

In both these cases, the checks are implemented incorrectly and can lead to a CORS bypass resulting in CVE-2023-28109 and CVE-2024-27302.

This PR aims to add a query, and its corresponding qhelp and tests for detecting the same vulnerability.

The databases to verify the same can be downloaded from

```
https://file.io/OQX8Q3H3hMd4
https://filetransfer.io/data-package/wAfSEvZu#link
```
---
 go/ql/src/experimental/CWE-639/urlCheck.ql | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 go/ql/src/experimental/CWE-639/urlCheck.ql

diff --git a/go/ql/src/experimental/CWE-639/urlCheck.ql b/go/ql/src/experimental/CWE-639/urlCheck.ql
new file mode 100644
index 0000000000000..a8d882f7a63ad
--- /dev/null
+++ b/go/ql/src/experimental/CWE-639/urlCheck.ql
@@ -0,0 +1,67 @@
+/**
+ * @name Incorrect check on url
+ * @description If a CORS policy is configured to accept an origin value obtained from the request data, it can lead to a policy bypass.
+ * @kind path-problem
+ * @problem.severity warning
+ * @id go/cors-bypass
+ * @tags security
+ *       experimental
+ *       external/cwe/cwe-942
+ *       external/cwe/cwe-346
+ */
+
+import go
+import codeql.dataflow.TaintTracking
+import semmle.go.dataflow.TaintTracking
+
+bindingset[s]
+predicate mayBeCors(string s) { s.toLowerCase().matches(["%origin%", "%cors%"]) }
+
+class MaybeOrigin extends RemoteFlowSource {
+  MaybeOrigin() {
+    none()
+    // exists(RemoteFlowSource r |
+    //   // Any write where the variables name could suggest it has something to do with cors.
+    //   exists(Write w, Variable v |
+    //     mayBeCors(w.getLhs().getName())
+    //     or
+    //     v.getAWrite() = w and mayBeCors(v.getName())
+    //   |
+    //     w = r.getASuccessor*().asInstruction()
+    //   )
+    //   or
+    //   // Any argument or a receiver whose name could suggest it has something to do with cors.
+    //   exists(DataFlow::CallNode c, DataFlow::ArgumentNode a |
+    //     c.getArgument(_) = r.getASuccessor*()
+    //     or
+    //     c.getReceiver() = r.getASuccessor*() and
+    //     a.argumentOf(c.asExpr(), _) and
+    //     mayBeCors(a.getStringValue())
+    //   |
+    //     mayBeCors([c.getTarget().getName()])
+    //   )
+    // |
+    //   this = r
+    // )
+  }
+}
+
+module UrlFlow implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof MaybeOrigin }
+
+  predicate isSink(DataFlow::Node node) {
+    exists(DataFlow::CallNode mc |
+      mc.getTarget().hasQualifiedName("strings", "HasSuffix") and
+      not mc.getArgument(1).asExpr().(StringLit).getExactValue().matches(".%")
+    |
+      mc.getArgument(0) = node
+      // and node.
+    )
+  }
+}
+
+module Flow = TaintTracking::Global<UrlFlow>;
+
+from DataFlow::Node source, DataFlow::Node sink
+where Flow::flow(source, sink)
+select sink, "asd"