Merge pull request #542 from gagliardetto/cors-misconfig

Add query to detect CORS misconfiguration
This commit is contained in:
Chris Smowton
2021-07-16 16:12:15 +01:00
committed by GitHub
7 changed files with 546 additions and 0 deletions

View File

@@ -0,0 +1,64 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
Web browsers, by default, disallow cross-origin resource sharing via direct HTTP requests (i.e. using a JavaScript HTTP client).
Still, to satisfy some needs that arose with the growth of the web, an expedient was created to make exceptions possible.
CORS (Cross-origin resource sharing) is a mechanism that allows resources of a web endpoint (let's call it "Peer A")
to be accessed from another web page belonging to a different domain ("Peer B").
</p>
<p>
For that to happen, Peer A needs to make available its CORS configuration via special headers on the desired endpoint
via the OPTIONS method.
</p>
<p>
This configuration can also allow the inclusion of cookies on the cross-origin request,
(i.e. when the <code>Access-Control-Allow-Credentials</code> header is set to true)
meaning that Peer B can send a request to Peer A that will include the cookies as if the request was executed by the user.
</p>
<p>
That can have dangerous effects if Peer B origin is not restricted correctly.
An example of a dangerous scenario is when <code>Access-Control-Allow-Origin</code> header is set to a value gotten from the Peer B's request
(and not correctly validated), or is set to special values such as <code>*</code> or <code>null</code>.
The above values can allow any Peer B to send requests to the misconfigured Peer A on behalf of the user.
</p>
<p>
Example scenario:
User is client of a bank that has its API misconfigured to accept CORS requests from any domain.
When the user loads an evil page, the evil page sends a request to the bank's API to transfer all funds
to evil party's account.
Given that the user was already logged in to the bank website, and had its session cookies set,
the evil party's request succeeds.
</p>
</overview>
<recommendation>
<p>
When configuring CORS that allow credentials passing,
it's best not to use user-provided values for the allowed origins response header,
especially if the cookies grant session permissions on the user's account.
</p>
<p>
It also can be very dangerous to set the allowed origins to <code>null</code> (which can be bypassed).
</p>
</recommendation>
<example>
<p>
The first example shows a few possible CORS misconfiguration cases:
</p>
<sample src="CorsMisconfigurationBad.go"/>
<p>
The second example show better configurations:
</p>
<sample src="CorsMisconfigurationGood.go"/>
</example>
<references>
<li>
Reference 1: <a href="https://portswigger.net/web-security/cors">PortSwigger Web Security Academy on CORS</a>.
</li>
<li>
Reference 2: <a href="https://www.youtube.com/watch?v=wgkj4ZgxI4c">AppSec EU 2017 Exploiting CORS Misconfigurations For Bitcoins And Bounties by James Kettle</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,192 @@
/**
* @name CORS misconfiguration
* @description If a CORS policy is configured to accept an origin value obtained from the request data,
* or is set to `*` or `null`, and it allows credential sharing, then the users of the
* application are vulnerable to the same range of attacks as in XSS (credential stealing, etc.).
* @kind problem
* @problem.severity warning
* @id go/cors-misconfiguration
* @tags security
* external/cwe/cwe-942
* external/cwe/cwe-346
*/
import go
import semmle.go.security.InsecureFeatureFlag::InsecureFeatureFlag
/**
* A flag indicating a check for satisfied permissions or test configuration.
*/
class AllowedFlag extends FlagKind {
AllowedFlag() { this = "allowed" }
bindingset[result]
override string getAFlagName() {
result.regexpMatch("(?i).*(allow|match|check|debug|devel|insecure).*")
}
}
/**
* Provides the name of the `Access-Control-Allow-Origin` header key.
*/
string headerAllowOrigin() { result = "Access-Control-Allow-Origin".toLowerCase() }
/**
* Provides the name of the `Access-Control-Allow-Credentials` header key.
*/
string headerAllowCredentials() { result = "Access-Control-Allow-Credentials".toLowerCase() }
/**
* An `Access-Control-Allow-Origin` header write.
*/
class AllowOriginHeaderWrite extends HTTP::HeaderWrite {
AllowOriginHeaderWrite() { this.getHeaderName() = headerAllowOrigin() }
}
/**
* An `Access-Control-Allow-Credentials` header write.
*/
class AllowCredentialsHeaderWrite extends HTTP::HeaderWrite {
AllowCredentialsHeaderWrite() { this.getHeaderName() = headerAllowCredentials() }
}
/**
* A taint-tracking configuration for reasoning about when an UntrustedFlowSource
* flows to a HeaderWrite that writes an `Access-Control-Allow-Origin` header's value.
*/
class FlowsUntrustedToAllowOriginHeader extends TaintTracking::Configuration {
FlowsUntrustedToAllowOriginHeader() { this = "from-untrusted-to-allow-origin-header-value" }
override predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
predicate isSink(DataFlow::Node sink, AllowOriginHeaderWrite hw) { sink = hw.getValue() }
override predicate isSanitizer(DataFlow::Node node) {
exists(ControlFlow::ConditionGuardNode cgn |
cgn.ensures(any(AllowedFlag f).getAFlag().getANode(), _)
|
cgn.dominates(node.getBasicBlock())
)
}
override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
}
/**
* Holds if the provided `allowOriginHW` HeaderWrite's parent ResponseWriter
* also has another HeaderWrite that sets a `Access-Control-Allow-Credentials`
* header to `true`.
*/
predicate allowCredentialsIsSetToTrue(AllowOriginHeaderWrite allowOriginHW) {
exists(AllowCredentialsHeaderWrite allowCredentialsHW |
allowCredentialsHW.getHeaderValue().toLowerCase() = "true"
|
allowOriginHW.getResponseWriter() = allowCredentialsHW.getResponseWriter()
)
}
/**
* Holds if the provided `allowOriginHW` HeaderWrite's value is set using an
* UntrustedFlowSource.
* The `message` parameter is populated with the warning message to be returned by the query.
*/
predicate flowsFromUntrustedToAllowOrigin(AllowOriginHeaderWrite allowOriginHW, string message) {
exists(FlowsUntrustedToAllowOriginHeader cfg, DataFlow::Node sink |
cfg.hasFlowTo(sink) and
cfg.isSink(sink, allowOriginHW)
|
message =
headerAllowOrigin() + " header is set to a user-defined value, and " +
headerAllowCredentials() + " is set to `true`"
)
}
/**
* Holds if the provided `allowOriginHW` HeaderWrite is for a `Access-Control-Allow-Origin`
* header and the value is set to `null`.
*/
predicate allowOriginIsNull(AllowOriginHeaderWrite allowOriginHW, string message) {
allowOriginHW.getHeaderValue().toLowerCase() = "null" and
message =
headerAllowOrigin() + " header is set to `" + allowOriginHW.getHeaderValue() + "`, and " +
headerAllowCredentials() + " is set to `true`"
}
/**
* A read on a map type.
*/
class MapRead extends DataFlow::ElementReadNode {
MapRead() { this.getBase().getType() instanceof MapType }
}
/**
* A taint-tracking configuration for reasoning about when an UntrustedFlowSource
* flows somewhere.
*/
class FlowsFromUntrusted extends TaintTracking::Configuration {
FlowsFromUntrusted() { this = "from-untrusted" }
override predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
predicate isSink(DataFlow::Node sink, ControlFlow::ConditionGuardNode cgn) {
exists(IfStmt ifs |
exists(Expr operand |
operand = ifs.getCond().getAChildExpr*() and
(
exists(DataFlow::CallExpr call | call = operand |
call.getTarget().hasQualifiedName("strings", "HasSuffix") and
sink.asExpr() = call.getArgument(0)
)
or
exists(MapRead mapRead |
operand = mapRead.asExpr() and
sink = mapRead.getIndex().getAPredecessor*()
// TODO: add _, ok : map[untrusted]; ok
)
or
exists(EqlExpr comp |
operand = comp and
(
sink.asExpr() = comp.getLeftOperand() and
not comp.getRightOperand().(StringLit).getStringValue() = ""
or
sink.asExpr() = comp.getRightOperand() and
not comp.getLeftOperand().(StringLit).getStringValue() = ""
)
)
)
)
|
cgn.getCondition() = ifs.getCond()
)
}
}
/**
* Holds if the provided `dst` is also destination of a `UntrustedFlowSource`.
*/
predicate flowsToGuardedByCheckOnUntrusted(AllowOriginHeaderWrite allowOriginHW) {
exists(FlowsFromUntrusted cfg, DataFlow::Node sink, ControlFlow::ConditionGuardNode cgn |
cfg.hasFlowTo(sink) and cfg.isSink(sink, cgn)
|
cgn.dominates(allowOriginHW.getBasicBlock())
)
}
from AllowOriginHeaderWrite allowOriginHW, string message
where
allowCredentialsIsSetToTrue(allowOriginHW) and
(
flowsFromUntrustedToAllowOrigin(allowOriginHW, message)
or
allowOriginIsNull(allowOriginHW, message)
) and
not flowsToGuardedByCheckOnUntrusted(allowOriginHW) and
not exists(ControlFlow::ConditionGuardNode cgn |
cgn.ensures(any(AllowedFlag f).getAFlag().getANode(), _)
|
cgn.dominates(allowOriginHW.getBasicBlock())
)
select allowOriginHW, message

View File

@@ -0,0 +1,24 @@
package main
import "net/http"
func main() {}
func bad1() {
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// BAD: 'null' origin is allowed,
// and Access-Control-Allow-Credentials is set to 'true'.
w.Header().Set("Access-Control-Allow-Origin", "null")
w.Header().Set("Access-Control-Allow-Credentials", "true")
})
}
func bad2() {
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// BAD: the `Access-Control-Allow-Origin` header is set using a user-defined value,
// and `Access-Control-Allow-Credentials` is set to 'true':
origin := req.Header.Get("origin")
w.Header().Set("Access-Control-Allow-Origin", origin)
w.Header().Set("Access-Control-Allow-Credentials", "true")
})
}

View File

@@ -0,0 +1,13 @@
package main
import "net/http"
func good1() {
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK-ish: the `Access-Control-Allow-Origin` header is set using a user-defined value,
// BUT `Access-Control-Allow-Credentials` is set to 'false':
origin := req.Header.Get("origin")
w.Header().Set("Access-Control-Allow-Origin", origin)
w.Header().Set("Access-Control-Allow-Credentials", "false")
})
}

View File

@@ -0,0 +1,5 @@
| CorsMisconfiguration.go:26:4:26:56 | call to Set | access-control-allow-origin header is set to `null`, and access-control-allow-credentials is set to `true` |
| CorsMisconfiguration.go:32:4:32:42 | call to Set | access-control-allow-origin header is set to `null`, and access-control-allow-credentials is set to `true` |
| CorsMisconfiguration.go:53:4:53:44 | call to Set | access-control-allow-origin header is set to a user-defined value, and access-control-allow-credentials is set to `true` |
| CorsMisconfiguration.go:60:4:60:56 | call to Set | access-control-allow-origin header is set to a user-defined value, and access-control-allow-credentials is set to `true` |
| CorsMisconfiguration.go:67:5:67:57 | call to Set | access-control-allow-origin header is set to a user-defined value, and access-control-allow-credentials is set to `true` |

View File

@@ -0,0 +1,247 @@
package main
import (
"net/http"
"strings"
)
const (
HeaderAllowOrigin = "Access-Control-Allow-Origin"
HeaderAllowCredentials = "Access-Control-Allow-Credentials"
)
func main() {
{
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK: all origins are allowed,
// but Access-Control-Allow-Credentials is not set.
w.Header().Set("Access-Control-Allow-Origin", "*")
})
}
{
const Null = "null"
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// BAD: 'null' origin is allowed,
// and Access-Control-Allow-Credentials is set to 'true'.
w.Header().Set("Access-Control-Allow-Origin", "null")
w.Header().Set("Access-Control-Allow-Credentials", "true")
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// BAD: 'null' origin is allowed,
// and `Access-Control-Allow-Credentials` is set to 'true':
w.Header().Set(HeaderAllowOrigin, Null)
w.Header().Set("Access-Control-Allow-Credentials", "true")
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK: 'null' origin is allowed,
// but Access-Control-Allow-Credentials is set to 'false'.
w.Header().Set("Access-Control-Allow-Origin", Null)
w.Header().Set("Access-Control-Allow-Credentials", "false")
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK: 'null' origin is allowed,
// but Access-Control-Allow-Credentials is not set.
w.Header().Set("Access-Control-Allow-Origin", Null)
})
}
{
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// BAD: the `Access-Control-Allow-Origin` header is set using a user-defined value,
// and `Access-Control-Allow-Credentials` is set to 'true':
origin := req.Header.Get("origin")
w.Header().Set(HeaderAllowOrigin, origin)
w.Header().Set("Access-Control-Allow-Credentials", "true")
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// BAD: the `Access-Control-Allow-Origin` header is set using a user-defined value,
// and `Access-Control-Allow-Credentials` is set to 'true':
origin := req.Header.Get("origin")
w.Header().Set("Access-Control-Allow-Origin", origin)
w.Header().Set(HeaderAllowCredentials, "true")
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// BAD: the `Access-Control-Allow-Origin` header is set using a user-defined value,
// and `Access-Control-Allow-Credentials` is set to 'true':
if origin := req.Header.Get("Origin"); origin != "" {
w.Header().Set("Access-Control-Allow-Origin", origin)
}
w.Header().Set(HeaderAllowCredentials, "true")
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK-ish: the `Access-Control-Allow-Origin` header is set using a user-defined value,
// BUT `Access-Control-Allow-Credentials` is set to 'false':
origin := req.Header.Get("origin")
w.Header().Set("Access-Control-Allow-Origin", origin)
w.Header().Set("Access-Control-Allow-Credentials", "false")
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK-ish: the `Access-Control-Allow-Origin` header is set using a user-defined value,
// BUT `Access-Control-Allow-Credentials` is not set:
origin := req.Header.Get("origin")
w.Header().Set("Access-Control-Allow-Origin", origin)
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK-ish: the `Access-Control-Allow-Origin` header is set using a user-defined value,
// and `Access-Control-Allow-Credentials` is set to 'true',
// BUT there is a check on the origin header:
origin := req.Header.Get("origin")
if allowedOrigin(origin) {
w.Header().Set("Access-Control-Allow-Origin", origin)
}
w.Header().Set(HeaderAllowCredentials, "true")
})
allowedOrigins := map[string]bool{
"example.com": true,
}
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK-ish: the `Access-Control-Allow-Origin` header is set using a user-defined value,
// and `Access-Control-Allow-Credentials` is set to 'true',
// BUT there is a check on the origin header:
origin := req.Header.Get("origin")
isAllowed := allowedOrigins[origin]
if isAllowed {
w.Header().Set("Access-Control-Allow-Origin", origin)
}
w.Header().Set(HeaderAllowCredentials, "true")
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK-ish: the `Access-Control-Allow-Origin` header is set using a user-defined value,
// and `Access-Control-Allow-Credentials` is set to 'true',
// BUT there is a check on the origin header:
origin := req.Header.Get("origin")
allowAllHosts := true
allowedHost := isAllowedHost([]string{"example.com"}, origin)
if allowAllHosts {
w.Header().Set("Access-Control-Allow-Origin", "*")
} else if allowedHost {
w.Header().Set("Access-Control-Allow-Origin", origin)
w.Header().Set(HeaderAllowCredentials, "true")
}
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK-ish: the input origin header is validated agains a whitelist.
responseHeader := w.Header()
{
origin := req.Header.Get("origin")
if origin != "" && origin != "null" {
if len(AccessControlAllowOrigins) == 0 ||
AccessControlAllowOrigins[origin] {
responseHeader.Set("Access-Control-Allow-Origin", origin)
responseHeader.Set("Access-Control-Allow-Credentials", "true")
}
} else {
responseHeader.Set("Access-Control-Allow-Origin", "*")
}
}
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
originSuffix := ".example.com"
// OK-ish: the input origin header is validated agains a suffix.
origin := req.Header.Get("Origin")
if origin != "" && (originSuffix == "" || strings.HasSuffix(origin, originSuffix)) {
w.Header().Set("Access-Control-Allow-Origin", origin)
w.Header().Set("Access-Control-Allow-Credentials", "true")
w.Header().Set("Access-Control-Allow-Headers", "Content-Type, x-requested-by, *")
w.Header().Set("Access-Control-Allow-Methods", "GET, PUT, POST, DELETE, OPTIONS")
if req.Method == http.MethodOptions {
w.WriteHeader(http.StatusOK)
return
}
}
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
originSuffix := ".example.com"
// OK-ish: the input origin header is validated agains a whitelist.
origin := req.Header.Get("Origin")
if origin != "" && (originSuffix == "" || AccessControlAllowOrigins[origin]) {
w.Header().Set("Access-Control-Allow-Origin", origin)
w.Header().Set("Access-Control-Allow-Credentials", "true")
w.Header().Set("Access-Control-Allow-Headers", "Content-Type, x-requested-by, *")
w.Header().Set("Access-Control-Allow-Methods", "GET, PUT, POST, DELETE, OPTIONS")
if req.Method == http.MethodOptions {
w.WriteHeader(http.StatusOK)
return
}
}
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK-ish: the input origin header is validated agains a whitelist.
origin := req.Header.Get("origin")
if origin != "" && origin != "null" {
if len(AccessControlAllowOrigins) == 0 || AccessControlAllowOrigins[origin] {
w.Header().Set("Access-Control-Allow-Origin", origin)
w.Header().Set("Access-Control-Allow-Credentials", "true")
}
} else {
w.Header().Set("Access-Control-Allow-Origin", "*")
}
})
// http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// // OK-ish: the input origin header is validated agains a whitelist.
// origin := req.Header.Get("origin")
// if origin != "" && origin != "null" {
// if _, ok := AccessControlAllowOrigins[origin]; ok {
// w.Header().Set("Access-Control-Allow-Origin", origin)
// w.Header().Set("Access-Control-Allow-Credentials", "true")
// }
// } else {
// w.Header().Set("Access-Control-Allow-Origin", "*")
// }
// })
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK-ish: the input origin header is validated agains a whitelist.
if origin := req.Header.Get("Origin"); cors[origin] {
w.Header().Set("Access-Control-Allow-Origin", origin)
} else if len(origin) > 0 && cors["*"] {
w.Header().Set("Access-Control-Allow-Origin", origin)
}
w.Header().Set("Access-Control-Allow-Methods", "POST, GET, OPTIONS")
w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Content-Length, Accept-Encoding, X-Token, X-Client")
w.Header().Set("Access-Control-Allow-Credentials", "true")
})
http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
// OK-ish: the input origin header is validated agains a whitelist.
origin := req.Header.Get("origin")
for _, v := range GetAllowOrigin() {
if v == origin {
w.Header().Add("Access-Control-Allow-Origin", origin)
break
}
}
w.Header().Set("Access-Control-Allow-Methods", "POST, GET, OPTIONS")
w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Content-Length, Accept-Encoding, X-Token, X-Client")
w.Header().Set("Access-Control-Allow-Credentials", "true")
})
}
}
var (
cors = map[string]bool{"*": true}
)
func GetAllowOrigin() []string {
return []string{
"example.com",
}
}
var AccessControlAllowOrigins map[string]bool
func isAllowedHost(allowed []string, try string) bool {
for _, v := range allowed {
if try == v {
return true
}
}
return false
}
func allowedOrigin(origin string) bool {
if origin == "example.com" {
return true
}
return false
}

View File

@@ -0,0 +1 @@
experimental/CWE-942/CorsMisconfiguration.ql