diff --git a/go/ql/lib/change-notes/2023-10-31-add-rs-cors-framework.md b/go/ql/lib/change-notes/2023-10-31-add-rs-cors-framework.md new file mode 100644 index 000000000000..3f2f7be82a5c --- /dev/null +++ b/go/ql/lib/change-notes/2023-10-31-add-rs-cors-framework.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added the [rs cors](https://github.com/rs/cors) library to the CorsMisconfiguration.ql query \ No newline at end of file diff --git a/go/ql/lib/go.qll b/go/ql/lib/go.qll index 8d3955e4dad4..27509e0e4bbb 100644 --- a/go/ql/lib/go.qll +++ b/go/ql/lib/go.qll @@ -32,6 +32,8 @@ import semmle.go.frameworks.Afero import semmle.go.frameworks.AwsLambda import semmle.go.frameworks.Beego import semmle.go.frameworks.BeegoOrm +import semmle.go.frameworks.Chi +import semmle.go.frameworks.RsCors import semmle.go.frameworks.Couchbase import semmle.go.frameworks.Echo import semmle.go.frameworks.ElazarlGoproxy diff --git a/go/ql/lib/semmle/go/frameworks/GinCors.qll b/go/ql/lib/semmle/go/frameworks/GinCors.qll index 269b45b6a2d3..a1b2d8367c69 100644 --- a/go/ql/lib/semmle/go/frameworks/GinCors.qll +++ b/go/ql/lib/semmle/go/frameworks/GinCors.qll @@ -21,7 +21,7 @@ module GinCors { /** * A write to the value of Access-Control-Allow-Credentials header */ - class AllowCredentialsWrite extends DataFlow::ExprNode { + class AllowCredentialsWrite extends UniversalAllowCredentialsWrite { DataFlow::Node base; AllowCredentialsWrite() { @@ -35,12 +35,12 @@ module GinCors { /** * Get config struct holding header values */ - DataFlow::Node getBase() { result = base } + override DataFlow::Node getBase() { result = base } /** * Get config variable holding header values */ - GinConfig getConfig() { + override GinConfig getConfig() { exists(GinConfig gc | ( gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = @@ -55,7 +55,7 @@ module GinCors { /** * A write to the value of Access-Control-Allow-Origins header */ - class AllowOriginsWrite extends DataFlow::ExprNode { + class AllowOriginsWrite extends UniversalOriginWrite { DataFlow::Node base; AllowOriginsWrite() { @@ -69,12 +69,12 @@ module GinCors { /** * Get config struct holding header values */ - DataFlow::Node getBase() { result = base } + override DataFlow::Node getBase() { result = base } /** * Get config variable holding header values */ - GinConfig getConfig() { + override GinConfig getConfig() { exists(GinConfig gc | ( gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = @@ -89,7 +89,7 @@ module GinCors { /** * A write to the value of Access-Control-Allow-Origins of value "*", overriding AllowOrigins */ - class AllowAllOriginsWrite extends DataFlow::ExprNode { + class AllowAllOriginsWrite extends UniversalAllowAllOriginsWrite { DataFlow::Node base; AllowAllOriginsWrite() { @@ -103,12 +103,12 @@ module GinCors { /** * Get config struct holding header values */ - DataFlow::Node getBase() { result = base } + override DataFlow::Node getBase() { result = base } /** * Get config variable holding header values */ - GinConfig getConfig() { + override GinConfig getConfig() { exists(GinConfig gc | ( gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = diff --git a/go/ql/lib/semmle/go/frameworks/RsCors.qll b/go/ql/lib/semmle/go/frameworks/RsCors.qll new file mode 100644 index 000000000000..f963543f9a0a --- /dev/null +++ b/go/ql/lib/semmle/go/frameworks/RsCors.qll @@ -0,0 +1,166 @@ +/** + * Provides classes for modeling the `github.com/rs/cors` package. + */ + +import go + +/** + * Provides abstract class for modeling the Go CORS handler model origin write. + */ +abstract class UniversalOriginWrite extends DataFlow::ExprNode { + abstract DataFlow::Node getBase(); + + abstract Variable getConfig(); +} + +/** + * Provides abstract class for modeling the Go CORS handler model allow all origins write. + */ +abstract class UniversalAllowAllOriginsWrite extends DataFlow::ExprNode { + abstract DataFlow::Node getBase(); + + abstract Variable getConfig(); +} + +/** + * Provides abstract class for modeling the Go CORS handler model allow credentials write. + */ +abstract class UniversalAllowCredentialsWrite extends DataFlow::ExprNode { + abstract DataFlow::Node getBase(); + + abstract Variable getConfig(); +} + +/** + * Provides classes for modeling the `github.com/rs/cors` package. + */ +module RsCors { + /** Gets the package name `github.com/gin-gonic/gin`. */ + string packagePath() { result = package("github.com/rs/cors", "") } + + /** + * A new function create a new rs Handler + */ + class New extends Function { + New() { exists(Function f | f.hasQualifiedName(packagePath(), "New") | this = f) } + } + + /** + * A write to the value of Access-Control-Allow-Credentials header + */ + class AllowCredentialsWrite extends UniversalAllowCredentialsWrite { + DataFlow::Node base; + + AllowCredentialsWrite() { + exists(Field f, Write w | + f.hasQualifiedName(packagePath(), "Options", "AllowCredentials") and + w.writesField(base, f, this) and + this.getType() instanceof BoolType + ) + } + + /** + * Get options struct holding header values + */ + override DataFlow::Node getBase() { result = base } + + /** + * Get options variable holding header values + */ + override RsOptions getConfig() { + exists(RsOptions gc | + ( + gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = + base.asInstruction() or + gc.getV().getAUse() = base + ) and + result = gc + ) + } + } + + /** + * A write to the value of Access-Control-Allow-Origins header + */ + class AllowOriginsWrite extends UniversalOriginWrite { + DataFlow::Node base; + + AllowOriginsWrite() { + exists(Field f, Write w | + f.hasQualifiedName(packagePath(), "Options", "AllowedOrigins") and + w.writesField(base, f, this) and + this.asExpr() instanceof SliceLit + ) + } + + /** + * Get options struct holding header values + */ + override DataFlow::Node getBase() { result = base } + + /** + * Get options variable holding header values + */ + override RsOptions getConfig() { + exists(RsOptions gc | + ( + gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = + base.asInstruction() or + gc.getV().getAUse() = base + ) and + result = gc + ) + } + } + + /** + * A write to the value of Access-Control-Allow-Origins of value "*", overriding AllowOrigins + */ + class AllowAllOriginsWrite extends UniversalAllowAllOriginsWrite { + DataFlow::Node base; + + AllowAllOriginsWrite() { + exists(Field f, Write w | + f.hasQualifiedName(packagePath(), "Options", "AllowAllOrigins") and + w.writesField(base, f, this) and + this.getType() instanceof BoolType + ) + } + + /** + * Get options struct holding header values + */ + override DataFlow::Node getBase() { result = base } + + /** + * Get options variable holding header values + */ + override RsOptions getConfig() { + exists(RsOptions gc | + ( + gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = + base.asInstruction() or + gc.getV().getAUse() = base + ) and + result = gc + ) + } + } + + /** + * A variable of type Options that holds the headers to be set. + */ + class RsOptions extends Variable { + SsaWithFields v; + + RsOptions() { + this = v.getBaseVariable().getSourceVariable() and + exists(Type t | t.hasQualifiedName(packagePath(), "Options") | v.getType() = t) + } + + /** + * Get variable declaration of Options + */ + SsaWithFields getV() { result = v } + } +} diff --git a/go/ql/src/experimental/CWE-942/CorsMisconfiguration.ql b/go/ql/src/experimental/CWE-942/CorsMisconfiguration.ql index 7a1ff256be1a..342f1addfe06 100644 --- a/go/ql/src/experimental/CWE-942/CorsMisconfiguration.ql +++ b/go/ql/src/experimental/CWE-942/CorsMisconfiguration.ql @@ -72,7 +72,7 @@ module UntrustedToAllowOriginHeaderConfig implements DataFlow::ConfigSig { module UntrustedToAllowOriginConfigConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } - additional predicate isSinkWrite(DataFlow::Node sink, GinCors::AllowOriginsWrite w) { sink = w } + additional predicate isSinkWrite(DataFlow::Node sink, UniversalOriginWrite w) { sink = w } predicate isSink(DataFlow::Node sink) { isSinkWrite(sink, _) } } @@ -102,17 +102,17 @@ predicate allowCredentialsIsSetToTrue(DataFlow::ExprNode allowOriginHW) { allowCredentialsHW.getResponseWriter() ) or - exists(GinCors::AllowCredentialsWrite allowCredentialsGin | + exists(UniversalAllowCredentialsWrite allowCredentialsGin | allowCredentialsGin.getExpr().getBoolValue() = true | - allowCredentialsGin.getConfig() = allowOriginHW.(GinCors::AllowOriginsWrite).getConfig() and - not exists(GinCors::AllowAllOriginsWrite allowAllOrigins | + allowCredentialsGin.getConfig() = allowOriginHW.(UniversalOriginWrite).getConfig() and + not exists(UniversalAllowAllOriginsWrite allowAllOrigins | allowAllOrigins.getExpr().getBoolValue() = true and allowCredentialsGin.getConfig() = allowAllOrigins.getConfig() ) or - allowCredentialsGin.getBase() = allowOriginHW.(GinCors::AllowOriginsWrite).getBase() and - not exists(GinCors::AllowAllOriginsWrite allowAllOrigins | + allowCredentialsGin.getBase() = allowOriginHW.(UniversalOriginWrite).getBase() and + not exists(UniversalAllowAllOriginsWrite allowAllOrigins | allowAllOrigins.getExpr().getBoolValue() = true and allowCredentialsGin.getBase() = allowAllOrigins.getBase() ) @@ -150,7 +150,7 @@ predicate allowOriginIsNull(DataFlow::ExprNode allowOriginHW, string message) { + " is set to `true`" or allowOriginHW - .(GinCors::AllowOriginsWrite) + .(UniversalOriginWrite) .asExpr() .(SliceLit) .getAnElement() diff --git a/go/ql/test/experimental/CWE-942/CorsMisconfiguration.expected b/go/ql/test/experimental/CWE-942/CorsMisconfiguration.expected index 060763358a5a..f542f2d098fb 100644 --- a/go/ql/test/experimental/CWE-942/CorsMisconfiguration.expected +++ b/go/ql/test/experimental/CWE-942/CorsMisconfiguration.expected @@ -5,3 +5,4 @@ | 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` | +| RsCors.go:11:21:11:59 | slice literal | access-control-allow-origin header is set to `null`, and access-control-allow-credentials is set to `true` | diff --git a/go/ql/test/experimental/CWE-942/RsCors.go b/go/ql/test/experimental/CWE-942/RsCors.go new file mode 100644 index 000000000000..dfe56f97b89f --- /dev/null +++ b/go/ql/test/experimental/CWE-942/RsCors.go @@ -0,0 +1,39 @@ +package main + +import ( + "net/http" + + "github.com/rs/cors" +) + +func rs_vulnerable() { + c := cors.New(cors.Options{ + AllowedOrigins: []string{"null", "http://foo.com:8080"}, + AllowCredentials: true, + // Enable Debugging for testing, consider disabling in production + Debug: true, + }) + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte("{\"hello\": \"world\"}")) + }) + + http.ListenAndServe(":8080", c.Handler(handler)) +} + +func rs_safe() { + c := cors.New(cors.Options{ + AllowedOrigins: []string{"http://foo.com:8080"}, + AllowCredentials: true, + // Enable Debugging for testing, consider disabling in production + Debug: true, + }) + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte("{\"hello\": \"world\"}")) + }) + + http.ListenAndServe(":8080", c.Handler(handler)) +} diff --git a/go/ql/test/experimental/CWE-942/go.mod b/go/ql/test/experimental/CWE-942/go.mod index d7e4401035ea..3655268ea3ea 100644 --- a/go/ql/test/experimental/CWE-942/go.mod +++ b/go/ql/test/experimental/CWE-942/go.mod @@ -5,6 +5,7 @@ go 1.21 require ( github.com/gin-contrib/cors v1.4.0 github.com/gin-gonic/gin v1.9.1 + github.com/rs/cors v1.10.1 ) require ( diff --git a/go/ql/test/experimental/CWE-942/vendor/github.com/rs/cors/stub.go b/go/ql/test/experimental/CWE-942/vendor/github.com/rs/cors/stub.go new file mode 100644 index 000000000000..007ee5d86e22 --- /dev/null +++ b/go/ql/test/experimental/CWE-942/vendor/github.com/rs/cors/stub.go @@ -0,0 +1,53 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for github.com/rs/cors, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: github.com/rs/cors (exports: Options; functions: New) + +// Package cors is a stub of github.com/rs/cors, generated by depstubber. +package cors + +import ( + http "net/http" +) + +type Cors struct { + Log Logger +} + +func (_ *Cors) Handler(_ http.Handler) http.Handler { + return nil +} + +func (_ *Cors) HandlerFunc(_ http.ResponseWriter, _ *http.Request) {} + +func (_ *Cors) OriginAllowed(_ *http.Request) bool { + return false +} + +func (_ *Cors) ServeHTTP(_ http.ResponseWriter, _ *http.Request, _ http.HandlerFunc) {} + +type Logger interface { + Printf(_ string, _ ...interface{}) +} + +func New(_ Options) *Cors { + return nil +} + +type Options struct { + AllowedOrigins []string + AllowOriginFunc func(string) bool + AllowOriginRequestFunc func(*http.Request, string) bool + AllowOriginVaryRequestFunc func(*http.Request, string) (bool, []string) + AllowedMethods []string + AllowedHeaders []string + ExposedHeaders []string + MaxAge int + AllowCredentials bool + AllowPrivateNetwork bool + OptionsPassthrough bool + OptionsSuccessStatus int + Debug bool + Logger Logger +} diff --git a/go/ql/test/experimental/CWE-942/vendor/modules.txt b/go/ql/test/experimental/CWE-942/vendor/modules.txt index 5be25b14ce1d..23f9ab8cc334 100644 --- a/go/ql/test/experimental/CWE-942/vendor/modules.txt +++ b/go/ql/test/experimental/CWE-942/vendor/modules.txt @@ -4,6 +4,9 @@ github.com/gin-contrib/cors # github.com/gin-gonic/gin v1.9.1 ## explicit github.com/gin-gonic/gin +# github.com/rs/cors v1.10.1 +## explicit +github.com/rs/cors # github.com/bytedance/sonic v1.9.1 ## explicit github.com/bytedance/sonic