Skip to content

Commit e078268

Browse files
KwstubbsKwstubbs
committed
Added gin cors framework
1 parent dafcd5e commit e078268

File tree

8 files changed

+1039
-52
lines changed

8 files changed

+1039
-52
lines changed

‎go/ql/lib/semmle/go/frameworks/GinCors.qll

Lines changed: 56 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,80 +1,100 @@
11
/**
2-
* Provides classes for working with untrusted flow sources from the `github.com/gin-gonic/gin` package.
2+
* Provides classes for working with untrusted flow sources from the `github.com/gin-contrib/cors` package.
33
*/
44

5-
import go
5+
import go
66

7-
module GinCors {
8-
/** Gets the package name `github.com/gin-gonic/gin`. */
9-
string packagePath() { result = package("github.com/gin-contrib/cors", "") }
10-
11-
/**
12-
* Data from a `Context` struct, considered as a source of untrusted flow.
13-
*/
14-
class New extends Function{
15-
New() {
16-
exists(Function f | f.hasQualifiedName(packagePath(), "New") | this = f)
17-
}
7+
module GinCors {
8+
/** Gets the package name `github.com/gin-gonic/gin`. */
9+
string packagePath() { result = package("github.com/gin-contrib/cors", "") }
1810

19-
}
20-
class Config extends Variable {
21-
Config() { this.hasQualifiedName(packagePath(), "Config") }
22-
23-
// Field getAllowCredentials() {
24-
// result = this.getField("AllowCredentials")
25-
// }
11+
/**
12+
* New function create a new gin Handler that passed to gin as middleware
13+
*/
14+
class New extends Function {
15+
New() { exists(Function f | f.hasQualifiedName(packagePath(), "New") | this = f) }
2616
}
27-
class AllowCredentialsWrite extends DataFlow::ExprNode {
17+
18+
/**
19+
* A write to the value of Access-Control-Allow-Credentials header
20+
*/
21+
class AllowCredentialsWrite extends DataFlow::ExprNode {
2822
DataFlow::Node base;
2923
GinConfig gc;
24+
3025
AllowCredentialsWrite() {
3126
exists(Field f, Write w |
3227
f.hasQualifiedName(packagePath(), "Config", "AllowCredentials") and
3328
w.writesField(base, f, this) and
3429
this.getType() instanceof BoolType and
35-
(gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = base.asInstruction() or
36-
gc.getV().getAUse() = base)
30+
(
31+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
32+
base.asInstruction() or
33+
gc.getV().getAUse() = base
34+
)
3735
)
3836
}
39-
GinConfig getConfig() { result = gc}
37+
38+
GinConfig getConfig() { result = gc }
4039
}
41-
class AllowOriginsWrite extends DataFlow::ExprNode {
40+
41+
/**
42+
* A write to the value of Access-Control-Allow-Origins header
43+
*/
44+
class AllowOriginsWrite extends DataFlow::ExprNode {
4245
DataFlow::Node base;
4346
GinConfig gc;
47+
4448
AllowOriginsWrite() {
4549
exists(Field f, Write w |
4650
f.hasQualifiedName(packagePath(), "Config", "AllowOrigins") and
4751
w.writesField(base, f, this) and
4852
this.asExpr() instanceof SliceLit and
49-
(gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() = base.asInstruction() or
50-
gc.getV().getAUse() = base)
53+
(
54+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
55+
base.asInstruction() or
56+
gc.getV().getAUse() = base
57+
)
5158
)
5259
}
53-
GinConfig getConfig() { result = gc}
60+
61+
GinConfig getConfig() { result = gc }
5462
}
63+
64+
/**
65+
* A write to the value of Access-Control-Allow-Origins to "*"
66+
*/
5567
class AllowAllOriginsWrite extends DataFlow::ExprNode {
5668
DataFlow::Node base;
69+
GinConfig gc;
5770

5871
AllowAllOriginsWrite() {
5972
exists(Field f, Write w |
6073
f.hasQualifiedName(packagePath(), "Config", "AllowAllOrigins") and
6174
w.writesField(base, f, this) and
62-
this.getType() instanceof BoolType
63-
//and this.asExpr().(SliceLit).getAnElement().toString().matches("%null%")
75+
this.getType() instanceof BoolType and
76+
(
77+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
78+
base.asInstruction() or
79+
gc.getV().getAUse() = base
80+
)
6481
)
6582
}
66-
DataFlow::Node getBase() { result = base}
83+
84+
GinConfig getConfig() { result = gc }
6785
}
6886

69-
class GinConfig extends Variable {
87+
/**
88+
* A variable of type Config that holds the headers to be set.
89+
*/
90+
class GinConfig extends Variable {
7091
SsaWithFields v;
7192

7293
GinConfig() {
7394
this = v.getBaseVariable().getSourceVariable() and
7495
exists(Type t | t.hasQualifiedName(packagePath(), "Config") | v.getType() = t)
7596
}
76-
SsaWithFields getV() { result = v}
97+
98+
SsaWithFields getV() { result = v }
7799
}
78-
79-
}
80-
100+
}

‎go/ql/src/experimental/CWE-942/CorsMisconfiguration.ql

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,7 @@ module UntrustedToAllowOriginHeaderConfig implements DataFlow::ConfigSig {
7272
module UntrustedToAllowOriginConfigConfig implements DataFlow::ConfigSig {
7373
predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
7474

75-
additional predicate isSinkWrite(DataFlow::Node sink, GinCors::AllowOriginsWrite w) {
76-
sink = w
77-
}
75+
additional predicate isSinkWrite(DataFlow::Node sink, GinCors::AllowOriginsWrite w) { sink = w }
7876

7977
predicate isSink(DataFlow::Node sink) { isSinkWrite(sink, _) }
8078
}
@@ -96,17 +94,18 @@ predicate allowCredentialsIsSetToTrue(DataFlow::ExprNode allowOrigin) {
9694
exists(AllowCredentialsHeaderWrite allowCredentialsHW |
9795
allowCredentialsHW.getHeaderValue().toLowerCase() = "true"
9896
|
99-
allowOrigin.(AllowOriginHeaderWrite).getResponseWriter() = allowCredentialsHW.getResponseWriter()
97+
allowOrigin.(AllowOriginHeaderWrite).getResponseWriter() =
98+
allowCredentialsHW.getResponseWriter()
10099
)
101100
or
102101
exists(GinCors::AllowCredentialsWrite allowCredentialsGin |
103-
allowCredentialsGin.toString() = "true"
104-
|
102+
allowCredentialsGin.toString() = "true"
103+
|
105104
//flow only goes in one direction so fix this before PR
106-
allowCredentialsGin.getConfig() = allowOrigin.(GinCors::AllowOriginsWrite).getConfig()
107-
and
105+
allowCredentialsGin.getConfig() = allowOrigin.(GinCors::AllowOriginsWrite).getConfig() and
108106
not exists(GinCors::AllowAllOriginsWrite allowAllOrigins |
109-
allowAllOrigins.toString() = "true" //and DataFlow::localFlow(allowCredentialsGin.getBase(), allowAllOrigins.getBase())
107+
allowAllOrigins.toString() = "true" and
108+
allowCredentialsGin.getConfig() = allowAllOrigins.getConfig()
110109
)
111110
)
112111
}
@@ -119,8 +118,9 @@ predicate allowCredentialsIsSetToTrue(DataFlow::ExprNode allowOrigin) {
119118
predicate flowsFromUntrustedToAllowOrigin(DataFlow::ExprNode allowOriginHW, string message) {
120119
exists(DataFlow::Node sink |
121120
UntrustedToAllowOriginHeaderFlow::flowTo(sink) and
122-
UntrustedToAllowOriginHeaderConfig::isSinkHW(sink, allowOriginHW) or
123-
UntrustedToAllowOriginConfigFlow::flowTo(sink) and
121+
UntrustedToAllowOriginHeaderConfig::isSinkHW(sink, allowOriginHW)
122+
or
123+
UntrustedToAllowOriginConfigFlow::flowTo(sink) and
124124
UntrustedToAllowOriginConfigConfig::isSinkWrite(sink, allowOriginHW)
125125
|
126126
message =
@@ -136,14 +136,22 @@ predicate flowsFromUntrustedToAllowOrigin(DataFlow::ExprNode allowOriginHW, stri
136136
predicate allowOriginIsNull(DataFlow::ExprNode allowOrigin, string message) {
137137
allowOrigin.(AllowOriginHeaderWrite).getHeaderValue().toLowerCase() = "null" and
138138
message =
139-
headerAllowOrigin() + " header is set to `" + allowOrigin.(AllowOriginHeaderWrite).getHeaderValue() + "`, and " +
140-
headerAllowCredentials() + " is set to `true`"
139+
headerAllowOrigin() + " header is set to `" +
140+
allowOrigin.(AllowOriginHeaderWrite).getHeaderValue() + "`, and " + headerAllowCredentials() +
141+
" is set to `true`"
141142
or
142-
allowOrigin.(GinCors::AllowOriginsWrite).asExpr().(SliceLit).getAnElement().toString().matches("\"null%\"") and
143+
allowOrigin
144+
.(GinCors::AllowOriginsWrite)
145+
.asExpr()
146+
.(SliceLit)
147+
.getAnElement()
148+
.toString()
149+
.toLowerCase()
150+
.matches("\"null\"") and
143151
message =
144-
headerAllowOrigin() + " header is set to `" + allowOrigin.(GinCors::AllowOriginsWrite).asExpr().(SliceLit).getAnElement().toString() + "`, and " +
152+
headerAllowOrigin() + " header is set to `" + "null" + "`, and " +
153+
//allowOrigin.(GinCors::AllowOriginsWrite).asExpr().(SliceLit).getAnElement().toString()
145154
headerAllowCredentials() + " is set to `true`"
146-
147155
}
148156

149157
/**

‎go/ql/test/experimental/CWE-942/CorsGin.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
package main
2+
3+
import (
4+
"net/http"
5+
"time"
6+
7+
"github.com/gin-contrib/cors"
8+
"github.com/gin-gonic/gin"
9+
)
10+
11+
/*
12+
** Function is vulnerable due to AllowAllOrigins = true aka Access-Control-Allow-Origin: null
13+
*/
14+
func vunlnerable() {
15+
router := gin.Default()
16+
// CORS for https://foo.com and null
17+
// - GET and POST methods
18+
// - Origin header
19+
// - Credentials share
20+
// - Preflight requests cached for 12 hours
21+
config_vulnerable := cors.Config{
22+
AllowMethods: []string{"GET", "POST"},
23+
AllowHeaders: []string{"Origin"},
24+
ExposeHeaders: []string{"Content-Length"},
25+
AllowCredentials: true,
26+
MaxAge: 12 * time.Hour,
27+
}
28+
config_vulnerable.AllowOrigins = []string{"null", "https://foo.com"}
29+
router.Use(cors.New(config_vulnerable))
30+
router.GET("/", func(c *gin.Context) {
31+
c.String(http.StatusOK, "hello world")
32+
})
33+
router.Run()
34+
}
35+
36+
/*
37+
** Function is safe due to hardcoded origin and AllowCredentials: true
38+
*/
39+
func safe() {
40+
router := gin.Default()
41+
// CORS for https://foo.com origin, allowing:
42+
// - GET and POST methods
43+
// - Origin header
44+
// - Credentials share
45+
// - Preflight requests cached for 12 hours
46+
config_safe := cors.Config{
47+
AllowMethods: []string{"GET", "POST"},
48+
AllowHeaders: []string{"Origin"},
49+
ExposeHeaders: []string{"Content-Length"},
50+
AllowCredentials: true,
51+
MaxAge: 12 * time.Hour,
52+
}
53+
config_safe.AllowOrigins = []string{"https://foo.com"}
54+
router.Use(cors.New(config_safe))
55+
router.GET("/", func(c *gin.Context) {
56+
c.String(http.StatusOK, "hello world")
57+
})
58+
router.Run()
59+
}
60+
61+
/*
62+
** Function is safe due to AllowAllOrigins = true aka Access-Control-Allow-Origin: *
63+
*/
64+
func AllowAllTrue() {
65+
router := gin.Default()
66+
// CORS for https://foo.com origin, allowing:
67+
// - PUT and PATCH methods
68+
// - Origin header
69+
// - Credentials share
70+
// - Preflight requests cached for 12 hours
71+
config_allowall := cors.Config{
72+
AllowMethods: []string{"GET", "POST"},
73+
AllowHeaders: []string{"Origin"},
74+
ExposeHeaders: []string{"Content-Length"},
75+
AllowCredentials: true,
76+
MaxAge: 12 * time.Hour,
77+
}
78+
config_allowall.AllowOrigins = []string{"null"}
79+
config_allowall.AllowAllOrigins = true
80+
router.Use(cors.New(config_allowall))
81+
router.GET("/", func(c *gin.Context) {
82+
c.String(http.StatusOK, "hello world")
83+
})
84+
router.Run()
85+
}

‎go/ql/test/experimental/CWE-942/CorsMisconfiguration.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
| CorsGin.go:28:35:28:69 | slice literal | access-control-allow-origin header is set to `null`, and access-control-allow-credentials is set to `true` |
12
| 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` |
23
| 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` |
34
| 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` |

‎go/ql/test/experimental/CWE-942/go.mod

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
module corsmiconfiguration/test
2+
3+
go 1.21
4+
5+
require (
6+
github.com/gin-contrib/cors v1.4.0
7+
github.com/gin-gonic/gin v1.9.1
8+
)
9+
10+
require (
11+
github.com/bytedance/sonic v1.9.1 // indirect
12+
github.com/chenzhuoyu/base64x v0.0.0-20221115062448-fe3a3abad311 // indirect
13+
github.com/gabriel-vasile/mimetype v1.4.2 // indirect
14+
github.com/gin-contrib/sse v0.1.0 // indirect
15+
github.com/go-playground/locales v0.14.1 // indirect
16+
github.com/go-playground/universal-translator v0.18.1 // indirect
17+
github.com/go-playground/validator/v10 v10.14.0 // indirect
18+
github.com/goccy/go-json v0.10.2 // indirect
19+
github.com/json-iterator/go v1.1.12 // indirect
20+
github.com/klauspost/cpuid/v2 v2.2.4 // indirect
21+
github.com/leodido/go-urn v1.2.4 // indirect
22+
github.com/mattn/go-isatty v0.0.19 // indirect
23+
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
24+
github.com/modern-go/reflect2 v1.0.2 // indirect
25+
github.com/pelletier/go-toml/v2 v2.0.8 // indirect
26+
github.com/twitchyliquid64/golang-asm v0.15.1 // indirect
27+
github.com/ugorji/go/codec v1.2.11 // indirect
28+
golang.org/x/arch v0.3.0 // indirect
29+
golang.org/x/crypto v0.9.0 // indirect
30+
golang.org/x/net v0.10.0 // indirect
31+
golang.org/x/sys v0.8.0 // indirect
32+
golang.org/x/text v0.9.0 // indirect
33+
google.golang.org/protobuf v1.30.0 // indirect
34+
gopkg.in/yaml.v3 v3.0.1 // indirect
35+
)

‎go/ql/test/experimental/CWE-942/vendor/github.com/gin-contrib/cors/stub.go

Lines changed: 41 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)