diff --git a/ql/src/experimental/CWE-942/CorsMisconfiguration.ql b/ql/src/experimental/CWE-942/CorsMisconfiguration.ql index 9099a6685a9..a2c6bd3ef08 100644 --- a/ql/src/experimental/CWE-942/CorsMisconfiguration.ql +++ b/ql/src/experimental/CWE-942/CorsMisconfiguration.ql @@ -12,6 +12,19 @@ */ 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. @@ -36,6 +49,14 @@ class FlowsUntrustedToAllowOriginHeader extends TaintTracking::Configuration { hw.getHeaderName() = headerAllowOrigin() and 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, _) } } @@ -88,5 +109,10 @@ where flowsFromUntrustedToAllowOrigin(allowOriginHW, message) or allowOriginIsWildcardOrNull(allowOriginHW, message) + ) and + not exists(ControlFlow::ConditionGuardNode cgn | + cgn.ensures(any(AllowedFlag f).getAFlag().getANode(), _) + | + cgn.dominates(allowOriginHW.getBasicBlock()) ) select allowOriginHW, message diff --git a/ql/test/experimental/CWE-942/CorsMisconfiguration.expected b/ql/test/experimental/CWE-942/CorsMisconfiguration.expected index c1bb884c613..40ab35a0d87 100644 --- a/ql/test/experimental/CWE-942/CorsMisconfiguration.expected +++ b/ql/test/experimental/CWE-942/CorsMisconfiguration.expected @@ -5,3 +5,4 @@ | CorsMisconfiguration.go:68:4:68:53 | call to Set | access-control-allow-origin header is set to `*`, and access-control-allow-credentials is set to `true` | | CorsMisconfiguration.go:86:4:86: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:93:4:93: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:100:5:100: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` | diff --git a/ql/test/experimental/CWE-942/CorsMisconfiguration.go b/ql/test/experimental/CWE-942/CorsMisconfiguration.go old mode 100755 new mode 100644 index 96517d42a4b..5570f3c243e --- a/ql/test/experimental/CWE-942/CorsMisconfiguration.go +++ b/ql/test/experimental/CWE-942/CorsMisconfiguration.go @@ -93,6 +93,14 @@ func main() { 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': @@ -106,5 +114,59 @@ func main() { 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") + }) } } + +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 +}