Skip to content

Commit

Permalink
Merge pull request #14873 from Kwstubbs/go-rs-cors
Browse files Browse the repository at this point in the history
Go: Add Rs Cors Support
  • Loading branch information
owen-mc authored Jan 20, 2025
2 parents 66777e6 + 217bc74 commit 4e59ac4
Show file tree
Hide file tree
Showing 10 changed files with 302 additions and 16 deletions.
4 changes: 4 additions & 0 deletions go/ql/lib/change-notes/2023-10-31-add-rs-cors-framework.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added the [rs cors](https://github.com/rs/cors) library to the CorsMisconfiguration.ql query
1 change: 1 addition & 0 deletions go/ql/lib/go.qll
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import semmle.go.frameworks.Afero
import semmle.go.frameworks.AwsLambda
import semmle.go.frameworks.Beego
import semmle.go.frameworks.BeegoOrm
import semmle.go.frameworks.RsCors
import semmle.go.frameworks.Couchbase
import semmle.go.frameworks.Echo
import semmle.go.frameworks.ElazarlGoproxy
Expand Down
18 changes: 9 additions & 9 deletions go/ql/lib/semmle/go/frameworks/GinCors.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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() =
Expand All @@ -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() {
Expand All @@ -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() =
Expand All @@ -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() {
Expand All @@ -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() =
Expand Down
184 changes: 184 additions & 0 deletions go/ql/lib/semmle/go/frameworks/RsCors.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
/**
* Provides classes for modeling the `github.com/rs/cors` package.
*/

import go

/**
* 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();
}

/**
* 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();
}

/**
* 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();
}

/**
* 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 }
}
}
14 changes: 7 additions & 7 deletions go/ql/src/experimental/CWE-942/CorsMisconfiguration.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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, _) }
}
Expand Down Expand Up @@ -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()
)
Expand Down Expand Up @@ -150,7 +150,7 @@ predicate allowOriginIsNull(DataFlow::ExprNode allowOriginHW, string message) {
+ " is set to `true`"
or
allowOriginHW
.(GinCors::AllowOriginsWrite)
.(UniversalOriginWrite)
.asExpr()
.(SliceLit)
.getAnElement()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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` |
39 changes: 39 additions & 0 deletions go/ql/test/experimental/CWE-942/RsCors.go
Original file line number Diff line number Diff line change
@@ -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))
}
1 change: 1 addition & 0 deletions go/ql/test/experimental/CWE-942/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
Loading

0 comments on commit 4e59ac4

Please sign in to comment.