Skip to content

Commit

Permalink
WIP: Go: CORS Bypass due to incorrect checks
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
porcupineyhairs committed Jun 23, 2024
1 parent a1743aa commit 3aacc85
Showing 1 changed file with 67 additions and 0 deletions.
67 changes: 67 additions & 0 deletions go/ql/src/experimental/CWE-639/urlCheck.ql
Original file line number Diff line number Diff line change
@@ -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"

0 comments on commit 3aacc85

Please sign in to comment.