From 3b78477406d9808ebddcd412db22b93a0ad902ab Mon Sep 17 00:00:00 2001 From: Kevin Stubbings Date: Sun, 5 Nov 2023 21:50:49 -0800 Subject: [PATCH 1/7] Basics --- go/ql/lib/change-notes/2023-10-31-add-rs-cors-framework.md | 4 ++++ go/ql/lib/go.qll | 1 + 2 files changed, 5 insertions(+) create mode 100644 go/ql/lib/change-notes/2023-10-31-add-rs-cors-framework.md 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 0af1bb9eeda0..fc2bff9f8c51 100644 --- a/go/ql/lib/go.qll +++ b/go/ql/lib/go.qll @@ -34,6 +34,7 @@ import semmle.go.frameworks.Afero 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 From 28288e0d232016529c4799047a9c3aa8d536af6d Mon Sep 17 00:00:00 2001 From: Kevin Stubbings Date: Sun, 5 Nov 2023 21:51:58 -0800 Subject: [PATCH 2/7] basic2 --- go/ql/lib/semmle/go/frameworks/RsCors.qll | 112 ++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 go/ql/lib/semmle/go/frameworks/RsCors.qll 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..043c7e422010 --- /dev/null +++ b/go/ql/lib/semmle/go/frameworks/RsCors.qll @@ -0,0 +1,112 @@ +/** + * Provides classes for modeling the `github.com/rs/cors` package. + */ + + import go + + /** + * 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 Handler that passed to handler chain as middleware + */ + 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 DataFlow::ExprNode { + RsOptions rs; + + AllowCredentialsWrite() { + exists(Field f, Write w, DataFlow::Node base | + f.hasQualifiedName(packagePath(), "Options", "AllowCredentials") and + w.writesField(base, f, this) and + this.getType() instanceof BoolType and + ( + rs.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = + base.asInstruction() or + rs.getV().getAUse() = base + ) + ) + } + + /** + * Get config variable holding header values + */ + RsOptions getConfig() { result = rs } + } + + /** + * A write to the value of Access-Control-Allow-Origins header + */ + class AllowOriginsWrite extends DataFlow::ExprNode { + RsOptions rs; + + AllowOriginsWrite() { + exists(Field f, Write w, DataFlow::Node base | + f.hasQualifiedName(packagePath(), "Options", "AllowedOrigins") and + w.writesField(base, f, this) and + this.asExpr() instanceof SliceLit and + ( + rs.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = + base.asInstruction() or + rs.getV().getAUse() = base + ) + ) + } + + /** + * Get config variable holding header values + */ + RsOptions getConfig() { result = rs } + } + + /** + * A write to the value of Access-Control-Allow-Origins of value "*", overriding AllowOrigins + */ + class AllowAllOriginsWrite extends DataFlow::ExprNode { + RsOptions rs; + + AllowAllOriginsWrite() { + exists(Field f, Write w, DataFlow::Node base | + f.hasQualifiedName(packagePath(), "Options", "AllowAllOrigins") and + w.writesField(base, f, this) and + this.getType() instanceof BoolType and + ( + rs.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = + base.asInstruction() or + rs.getV().getAUse() = base + ) + ) + } + + /** + * Get config variable holding header values + */ + RsOptions getConfig() { result = rs } + } + + /** + * A variable of type Config 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 RsOptions + */ + SsaWithFields getV() { result = v } + } + } \ No newline at end of file From 9958ad904c867db0d7f46cb8cd4e7e79cb0e2e07 Mon Sep 17 00:00:00 2001 From: Kevin Stubbings Date: Mon, 6 Nov 2023 14:35:26 -0800 Subject: [PATCH 3/7] thesame --- go/ql/lib/semmle/go/frameworks/RsCors.qll | 85 +++++++++++++++-------- 1 file changed, 56 insertions(+), 29 deletions(-) diff --git a/go/ql/lib/semmle/go/frameworks/RsCors.qll b/go/ql/lib/semmle/go/frameworks/RsCors.qll index 043c7e422010..cbbc1e78ad0d 100644 --- a/go/ql/lib/semmle/go/frameworks/RsCors.qll +++ b/go/ql/lib/semmle/go/frameworks/RsCors.qll @@ -12,7 +12,7 @@ string packagePath() { result = package("github.com/rs/cors", "") } /** - * A new function create a new Handler that passed to handler chain as middleware + * A new function create a new gin Handler that passed to gin as middleware */ class New extends Function { New() { exists(Function f | f.hasQualifiedName(packagePath(), "New") | this = f) } @@ -22,75 +22,102 @@ * A write to the value of Access-Control-Allow-Credentials header */ class AllowCredentialsWrite extends DataFlow::ExprNode { - RsOptions rs; + DataFlow::Node base; AllowCredentialsWrite() { - exists(Field f, Write w, DataFlow::Node base | + exists(Field f, Write w | f.hasQualifiedName(packagePath(), "Options", "AllowCredentials") and w.writesField(base, f, this) and - this.getType() instanceof BoolType and - ( - rs.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = - base.asInstruction() or - rs.getV().getAUse() = base - ) + this.getType() instanceof BoolType ) } + /** + * Get config struct holding header values + */ + DataFlow::Node getBase() { result = base } + /** * Get config variable holding header values */ - RsOptions getConfig() { result = rs } + 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 DataFlow::ExprNode { - RsOptions rs; + DataFlow::Node base; AllowOriginsWrite() { - exists(Field f, Write w, DataFlow::Node base | + exists(Field f, Write w | f.hasQualifiedName(packagePath(), "Options", "AllowedOrigins") and w.writesField(base, f, this) and - this.asExpr() instanceof SliceLit and - ( - rs.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = - base.asInstruction() or - rs.getV().getAUse() = base - ) + this.asExpr() instanceof SliceLit ) } + /** + * Get config struct holding header values + */ + DataFlow::Node getBase() { result = base } + /** * Get config variable holding header values */ - RsOptions getConfig() { result = rs } + 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 DataFlow::ExprNode { - RsOptions rs; + DataFlow::Node base; AllowAllOriginsWrite() { - exists(Field f, Write w, DataFlow::Node base | + exists(Field f, Write w | f.hasQualifiedName(packagePath(), "Options", "AllowAllOrigins") and w.writesField(base, f, this) and - this.getType() instanceof BoolType and - ( - rs.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = - base.asInstruction() or - rs.getV().getAUse() = base - ) + this.getType() instanceof BoolType ) } + /** + * Get config struct holding header values + */ + DataFlow::Node getBase() { result = base } + /** * Get config variable holding header values */ - RsOptions getConfig() { result = rs } + RsOptions getConfig() { + exists(RsOptions gc | + ( + gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = + base.asInstruction() or + gc.getV().getAUse() = base + ) and + result = gc + ) + } } /** @@ -105,7 +132,7 @@ } /** - * Get variable declaration of RsOptions + * Get variable declaration of GinConfig */ SsaWithFields getV() { result = v } } From d7e2fbc11d79ce7c266a11f1a2aedd9158d5423f Mon Sep 17 00:00:00 2001 From: Kevin Stubbings Date: Tue, 21 Nov 2023 14:27:17 -0800 Subject: [PATCH 4/7] Finish --- go/ql/lib/semmle/go/frameworks/GinCors.qll | 18 +- go/ql/lib/semmle/go/frameworks/RsCors.qll | 297 ++++++++++-------- .../CWE-942/CorsMisconfiguration.ql | 14 +- .../CWE-942/CorsMisconfiguration.expected | 1 + go/ql/test/experimental/CWE-942/RsCors.go | 39 +++ go/ql/test/experimental/CWE-942/go.mod | 1 + .../experimental/CWE-942/vendor/modules.txt | 3 + 7 files changed, 222 insertions(+), 151 deletions(-) create mode 100644 go/ql/test/experimental/CWE-942/RsCors.go 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 index cbbc1e78ad0d..f963543f9a0a 100644 --- a/go/ql/lib/semmle/go/frameworks/RsCors.qll +++ b/go/ql/lib/semmle/go/frameworks/RsCors.qll @@ -2,138 +2,165 @@ * Provides classes for modeling the `github.com/rs/cors` package. */ - import go - - /** - * 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 gin Handler that passed to gin as middleware - */ - 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 DataFlow::ExprNode { - 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 config struct holding header values - */ - DataFlow::Node getBase() { result = base } - - /** - * Get config variable holding header values - */ - 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 DataFlow::ExprNode { - 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 config struct holding header values - */ - DataFlow::Node getBase() { result = base } - - /** - * Get config variable holding header values - */ - 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 DataFlow::ExprNode { - 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 config struct holding header values - */ - DataFlow::Node getBase() { result = base } - - /** - * Get config variable holding header values - */ - 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 Config 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 GinConfig - */ - SsaWithFields getV() { result = v } - } - } \ No newline at end of file +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 1bcf05e214ac..ad4669c2f7fe 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 UntrustedFlowSource } - 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/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 From 8277c602ac6aef7049018a8ffb799c77e607125d Mon Sep 17 00:00:00 2001 From: Kevin Stubbings Date: Tue, 21 Nov 2023 14:31:52 -0800 Subject: [PATCH 5/7] depstubber --- .../CWE-942/vendor/github.com/rs/cors/stub.go | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 go/ql/test/experimental/CWE-942/vendor/github.com/rs/cors/stub.go 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 +} From e1c601dc52d55514a76fe2dd0e6ae0fcb7770950 Mon Sep 17 00:00:00 2001 From: Kevin Stubbings Date: Fri, 17 Jan 2025 10:18:59 -0800 Subject: [PATCH 6/7] oops --- go/ql/lib/go.qll | 1 - 1 file changed, 1 deletion(-) diff --git a/go/ql/lib/go.qll b/go/ql/lib/go.qll index 27509e0e4bbb..df725017dc85 100644 --- a/go/ql/lib/go.qll +++ b/go/ql/lib/go.qll @@ -32,7 +32,6 @@ 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 From 217bc74278345c6fc613fde4b8c431530736f101 Mon Sep 17 00:00:00 2001 From: Kevin Stubbings Date: Sun, 19 Jan 2025 22:43:14 -0800 Subject: [PATCH 7/7] Fix documentation --- go/ql/lib/semmle/go/frameworks/RsCors.qll | 24 ++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/go/ql/lib/semmle/go/frameworks/RsCors.qll b/go/ql/lib/semmle/go/frameworks/RsCors.qll index f963543f9a0a..aeeb45312c30 100644 --- a/go/ql/lib/semmle/go/frameworks/RsCors.qll +++ b/go/ql/lib/semmle/go/frameworks/RsCors.qll @@ -5,29 +5,47 @@ import go /** - * Provides abstract class for modeling the Go CORS handler model origin write. + * An abstract class for modeling the Go CORS handler model origin write. */ abstract class UniversalOriginWrite extends DataFlow::ExprNode { + /** + * Get config variable holding header values + */ abstract DataFlow::Node getBase(); + /** + * Get config variable holding header values + */ abstract Variable getConfig(); } /** - * Provides abstract class for modeling the Go CORS handler model allow all origins write. + * An abstract class for modeling the Go CORS handler model allow all origins write. */ abstract class UniversalAllowAllOriginsWrite extends DataFlow::ExprNode { + /** + * Get config variable holding header values + */ abstract DataFlow::Node getBase(); + /** + * Get config variable holding header values + */ abstract Variable getConfig(); } /** - * Provides abstract class for modeling the Go CORS handler model allow credentials write. + * An abstract class for modeling the Go CORS handler model allow credentials write. */ abstract class UniversalAllowCredentialsWrite extends DataFlow::ExprNode { + /** + * Get config struct holding header values + */ abstract DataFlow::Node getBase(); + /** + * Get config variable holding header values + */ abstract Variable getConfig(); }