Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Go: CORS Bypass due to incorrect checks #16813

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 97 additions & 0 deletions go/ql/src/experimental/CWE-639/urlCheck.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/**
* @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

bindingset[s]
private predicate mayBeCors(string s) {
s.toLowerCase().matches(["%origin%", "%cors%"]) and not s.toLowerCase().matches(["%original%"])

Check warning

Code scanning / CodeQL

Singleton set literal Warning

Singleton set literal can be replaced by its member.
}

/**
* An argument to a Gorilla's OriginValidator Function taken as a source
*/
class GorillaOriginFuncSource extends DataFlow::Node {
GorillaOriginFuncSource() {
exists(FuncDef f, DataFlow::CallNode c |
// Find a func passed to `AllowedOriginValdiator` as a validator.
// The string parameter supplied to the validator is a remote controlled string supplied in the origin header.
// `gh.AllowedOriginValidator(func(origin string) bool{})`
f.getParameter(0).getType() instanceof StringType and
f.getNumParameter() = 1 and
c.getTarget().hasQualifiedName("github.com/gorilla/handlers", "AllowedOriginValidator") and
c.getArgument(0).asExpr() = f
|
this = DataFlow::parameterNode(f.getParameter(0))
)
}
}

private class MaybeOrigin extends DataFlow::Node {
MaybeOrigin() {
exists(RemoteFlowSource r |
// Any write where the variables name could suggest it has something to do with cors.
exists(Write w |
// Take `origin` in `origin := r.Header.Get(header)` as a source.
mayBeCors(w.getLhs().getName()) and
TaintTracking::localTaint(r, w.getRhs())
|
this = r //or this.asInstruction() = w
)
or
// Take the receiver, call or call arguments as source when any of their names match `maybeCors`
exists(DataFlow::CallNode c, DataFlow::ArgumentNode a |
TaintTracking::localTaint(r, a) and
a.argumentOf(c.asExpr(), _) and
mayBeCors([a.getStringValue(), c.getTarget().getName()])
|
// When argument or receiver is `maybeCors` take them as origin source.
// When the call name is `maybeCors` take the result as origin source
// (this = c.getResult(_) or this = a)
this = r
)
)
}
}

private module UrlFlow implements DataFlow::ConfigSig {

Check warning

Code scanning / CodeQL

Data flow configuration module naming Warning

Modules implementing a data flow configuration should end in Config.
predicate isSource(DataFlow::Node node) {
node instanceof MaybeOrigin or node instanceof GorillaOriginFuncSource
}

predicate isSink(DataFlow::Node node) {
exists(DataFlow::CallNode mc, DataFlow::ArgumentNode a |
// Get a call to `strings.HasSuffix(origin, allowedDomain)`
mc.getTarget().hasQualifiedName("strings", "HasSuffix") and
a = mc.getArgument(1) and
// should not match ".domain.com"
not (
a.asExpr().(StringLit).getExactValue().matches(".%")
or
exists(AddExpr w | w.getLeftOperand().getStringValue().matches(".%") |
DataFlow::localFlow(DataFlow::exprNode(w), a)
)
)
|
mc.getArgument(0) = node
)
}
}

private module Flow = TaintTracking::Global<UrlFlow>;

private import Flow::PathGraph

from Flow::PathNode source, Flow::PathNode sink
where Flow::flowPath(source, sink)
select sink.getNode(), source, sink,
"This can lead to a Cross Origin Resource Sharing(CORS) policy bypass"
Loading