Skip to content

Commit 359dcf3

Browse files
owen-mcowen-mc
authored
Merge pull request #14649 from Kwstubbs/go-cors
Go: Add Cors Gin Support
2 parents 6f56a65 + 57c645b commit 359dcf3

File tree

11 files changed

+1185
-12
lines changed

11 files changed

+1185
-12
lines changed

‎go/ql/lib/change-notes/2023-10-31-add-gin-cors-framework.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added the [gin cors](https://github.com/gin-contrib/cors) library to the CorsMisconfiguration.ql query

‎go/ql/lib/go.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import semmle.go.frameworks.Email
4141
import semmle.go.frameworks.Encoding
4242
import semmle.go.frameworks.Fiber
4343
import semmle.go.frameworks.Gin
44+
import semmle.go.frameworks.GinCors
4445
import semmle.go.frameworks.Glog
4546
import semmle.go.frameworks.GoKit
4647
import semmle.go.frameworks.GoMicro

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

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
/**
2+
* Provides classes for modeling the `github.com/gin-contrib/cors` package.
3+
*/
4+
5+
import go
6+
7+
/**
8+
* Provides classes for modeling the `github.com/gin-contrib/cors` package.
9+
*/
10+
module GinCors {
11+
/** Gets the package name `github.com/gin-gonic/gin`. */
12+
string packagePath() { result = package("github.com/gin-contrib/cors", "") }
13+
14+
/**
15+
* A new function create a new gin Handler that passed to gin as middleware
16+
*/
17+
class New extends Function {
18+
New() { exists(Function f | f.hasQualifiedName(packagePath(), "New") | this = f) }
19+
}
20+
21+
/**
22+
* A write to the value of Access-Control-Allow-Credentials header
23+
*/
24+
class AllowCredentialsWrite extends DataFlow::ExprNode {
25+
DataFlow::Node base;
26+
27+
AllowCredentialsWrite() {
28+
exists(Field f, Write w |
29+
f.hasQualifiedName(packagePath(), "Config", "AllowCredentials") and
30+
w.writesField(base, f, this) and
31+
this.getType() instanceof BoolType
32+
)
33+
}
34+
35+
/**
36+
* Get config struct holding header values
37+
*/
38+
DataFlow::Node getBase() { result = base }
39+
40+
/**
41+
* Get config variable holding header values
42+
*/
43+
GinConfig getConfig() {
44+
exists(GinConfig gc |
45+
(
46+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
47+
base.asInstruction() or
48+
gc.getV().getAUse() = base
49+
) and
50+
result = gc
51+
)
52+
}
53+
}
54+
55+
/**
56+
* A write to the value of Access-Control-Allow-Origins header
57+
*/
58+
class AllowOriginsWrite extends DataFlow::ExprNode {
59+
DataFlow::Node base;
60+
61+
AllowOriginsWrite() {
62+
exists(Field f, Write w |
63+
f.hasQualifiedName(packagePath(), "Config", "AllowOrigins") and
64+
w.writesField(base, f, this) and
65+
this.asExpr() instanceof SliceLit
66+
)
67+
}
68+
69+
/**
70+
* Get config struct holding header values
71+
*/
72+
DataFlow::Node getBase() { result = base }
73+
74+
/**
75+
* Get config variable holding header values
76+
*/
77+
GinConfig getConfig() {
78+
exists(GinConfig gc |
79+
(
80+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
81+
base.asInstruction() or
82+
gc.getV().getAUse() = base
83+
) and
84+
result = gc
85+
)
86+
}
87+
}
88+
89+
/**
90+
* A write to the value of Access-Control-Allow-Origins of value "*", overriding AllowOrigins
91+
*/
92+
class AllowAllOriginsWrite extends DataFlow::ExprNode {
93+
DataFlow::Node base;
94+
95+
AllowAllOriginsWrite() {
96+
exists(Field f, Write w |
97+
f.hasQualifiedName(packagePath(), "Config", "AllowAllOrigins") and
98+
w.writesField(base, f, this) and
99+
this.getType() instanceof BoolType
100+
)
101+
}
102+
103+
/**
104+
* Get config struct holding header values
105+
*/
106+
DataFlow::Node getBase() { result = base }
107+
108+
/**
109+
* Get config variable holding header values
110+
*/
111+
GinConfig getConfig() {
112+
exists(GinConfig gc |
113+
(
114+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
115+
base.asInstruction() or
116+
gc.getV().getAUse() = base
117+
) and
118+
result = gc
119+
)
120+
}
121+
}
122+
123+
/**
124+
* A variable of type Config that holds the headers to be set.
125+
*/
126+
class GinConfig extends Variable {
127+
SsaWithFields v;
128+
129+
GinConfig() {
130+
this = v.getBaseVariable().getSourceVariable() and
131+
exists(Type t | t.hasQualifiedName(packagePath(), "Config") | v.getType() = t)
132+
}
133+
134+
/**
135+
* Get variable declaration of GinConfig
136+
*/
137+
SsaWithFields getV() { result = v }
138+
}
139+
}

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

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,22 +69,53 @@ module UntrustedToAllowOriginHeaderConfig implements DataFlow::ConfigSig {
6969
predicate isSink(DataFlow::Node sink) { isSinkHW(sink, _) }
7070
}
7171

72+
module UntrustedToAllowOriginConfigConfig implements DataFlow::ConfigSig {
73+
predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
74+
75+
additional predicate isSinkWrite(DataFlow::Node sink, GinCors::AllowOriginsWrite w) { sink = w }
76+
77+
predicate isSink(DataFlow::Node sink) { isSinkWrite(sink, _) }
78+
}
79+
7280
/**
7381
* Tracks taint flowfor reasoning about when an `UntrustedFlowSource` flows to
7482
* a `HeaderWrite` that writes an `Access-Control-Allow-Origin` header's value.
7583
*/
7684
module UntrustedToAllowOriginHeaderFlow = TaintTracking::Global<UntrustedToAllowOriginHeaderConfig>;
7785

86+
/**
87+
* Tracks taint flowfor reasoning about when an `UntrustedFlowSource` flows to
88+
* a `AllowOriginsWrite` that writes an `Access-Control-Allow-Origin` header's value.
89+
*/
90+
module UntrustedToAllowOriginConfigFlow = TaintTracking::Global<UntrustedToAllowOriginConfigConfig>;
91+
7892
/**
7993
* Holds if the provided `allowOriginHW` HeaderWrite's parent ResponseWriter
8094
* also has another HeaderWrite that sets a `Access-Control-Allow-Credentials`
8195
* header to `true`.
8296
*/
83-
predicate allowCredentialsIsSetToTrue(AllowOriginHeaderWrite allowOriginHW) {
97+
predicate allowCredentialsIsSetToTrue(DataFlow::ExprNode allowOriginHW) {
8498
exists(AllowCredentialsHeaderWrite allowCredentialsHW |
8599
allowCredentialsHW.getHeaderValue().toLowerCase() = "true"
86100
|
87-
allowOriginHW.getResponseWriter() = allowCredentialsHW.getResponseWriter()
101+
allowOriginHW.(AllowOriginHeaderWrite).getResponseWriter() =
102+
allowCredentialsHW.getResponseWriter()
103+
)
104+
or
105+
exists(GinCors::AllowCredentialsWrite allowCredentialsGin |
106+
allowCredentialsGin.getExpr().getBoolValue() = true
107+
|
108+
allowCredentialsGin.getConfig() = allowOriginHW.(GinCors::AllowOriginsWrite).getConfig() and
109+
not exists(GinCors::AllowAllOriginsWrite allowAllOrigins |
110+
allowAllOrigins.getExpr().getBoolValue() = true and
111+
allowCredentialsGin.getConfig() = allowAllOrigins.getConfig()
112+
)
113+
or
114+
allowCredentialsGin.getBase() = allowOriginHW.(GinCors::AllowOriginsWrite).getBase() and
115+
not exists(GinCors::AllowAllOriginsWrite allowAllOrigins |
116+
allowAllOrigins.getExpr().getBoolValue() = true and
117+
allowCredentialsGin.getBase() = allowAllOrigins.getBase()
118+
)
88119
)
89120
}
90121

@@ -93,10 +124,13 @@ predicate allowCredentialsIsSetToTrue(AllowOriginHeaderWrite allowOriginHW) {
93124
* UntrustedFlowSource.
94125
* The `message` parameter is populated with the warning message to be returned by the query.
95126
*/
96-
predicate flowsFromUntrustedToAllowOrigin(AllowOriginHeaderWrite allowOriginHW, string message) {
127+
predicate flowsFromUntrustedToAllowOrigin(DataFlow::ExprNode allowOriginHW, string message) {
97128
exists(DataFlow::Node sink |
98129
UntrustedToAllowOriginHeaderFlow::flowTo(sink) and
99130
UntrustedToAllowOriginHeaderConfig::isSinkHW(sink, allowOriginHW)
131+
or
132+
UntrustedToAllowOriginConfigFlow::flowTo(sink) and
133+
UntrustedToAllowOriginConfigConfig::isSinkWrite(sink, allowOriginHW)
100134
|
101135
message =
102136
headerAllowOrigin() + " header is set to a user-defined value, and " +
@@ -108,11 +142,23 @@ predicate flowsFromUntrustedToAllowOrigin(AllowOriginHeaderWrite allowOriginHW,
108142
* Holds if the provided `allowOriginHW` HeaderWrite is for a `Access-Control-Allow-Origin`
109143
* header and the value is set to `null`.
110144
*/
111-
predicate allowOriginIsNull(AllowOriginHeaderWrite allowOriginHW, string message) {
112-
allowOriginHW.getHeaderValue().toLowerCase() = "null" and
145+
predicate allowOriginIsNull(DataFlow::ExprNode allowOriginHW, string message) {
146+
allowOriginHW.(AllowOriginHeaderWrite).getHeaderValue().toLowerCase() = "null" and
147+
message =
148+
headerAllowOrigin() + " header is set to `" +
149+
allowOriginHW.(AllowOriginHeaderWrite).getHeaderValue() + "`, and " + headerAllowCredentials()
150+
+ " is set to `true`"
151+
or
152+
allowOriginHW
153+
.(GinCors::AllowOriginsWrite)
154+
.asExpr()
155+
.(SliceLit)
156+
.getAnElement()
157+
.getStringValue()
158+
.toLowerCase() = "null" and
113159
message =
114-
headerAllowOrigin() + " header is set to `" + allowOriginHW.getHeaderValue() + "`, and " +
115-
headerAllowCredentials() + " is set to `true`"
160+
headerAllowOrigin() + " header is set to `" + "null" + "`, and " + headerAllowCredentials() +
161+
" is set to `true`"
116162
}
117163

118164
/**
@@ -170,15 +216,15 @@ module FromUntrustedFlow = TaintTracking::Global<FromUntrustedConfig>;
170216
/**
171217
* Holds if the provided `allowOriginHW` is also destination of a `UntrustedFlowSource`.
172218
*/
173-
predicate flowsToGuardedByCheckOnUntrusted(AllowOriginHeaderWrite allowOriginHW) {
219+
predicate flowsToGuardedByCheckOnUntrusted(DataFlow::ExprNode allowOriginHW) {
174220
exists(DataFlow::Node sink, ControlFlow::ConditionGuardNode cgn |
175221
FromUntrustedFlow::flowTo(sink) and FromUntrustedConfig::isSinkCgn(sink, cgn)
176222
|
177223
cgn.dominates(allowOriginHW.getBasicBlock())
178224
)
179225
}
180226

181-
from AllowOriginHeaderWrite allowOriginHW, string message
227+
from DataFlow::ExprNode allowOriginHW, string message
182228
where
183229
allowCredentialsIsSetToTrue(allowOriginHW) and
184230
(

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

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
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+
// - PUT and PATCH methods
18+
// - Origin header
19+
// - Credentials share
20+
// - Preflight requests cached for 12 hours
21+
config_vulnerable := cors.Config{
22+
AllowMethods: []string{"PUT", "PATCH"},
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+
// - PUT and PATCH methods
43+
// - Origin header
44+
// - Credentials share
45+
// - Preflight requests cached for 12 hours
46+
config_safe := cors.Config{
47+
AllowMethods: []string{"PUT", "PATCH"},
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 "*" 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{"PUT", "PATCH"},
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+
}
86+
87+
func NoVariableVulnerable() {
88+
router := gin.Default()
89+
// CORS for https://foo.com origin, allowing:
90+
// - PUT and PATCH methods
91+
// - Origin header
92+
// - Credentials share
93+
// - Preflight requests cached for 12 hours
94+
router.Use(cors.New(cors.Config{
95+
AllowMethods: []string{"GET", "POST"},
96+
AllowHeaders: []string{"Origin"},
97+
ExposeHeaders: []string{"Content-Length"},
98+
AllowOrigins: []string{"null", "https://foo.com"},
99+
AllowCredentials: true,
100+
MaxAge: 12 * time.Hour,
101+
}))
102+
router.GET("/", func(c *gin.Context) {
103+
c.String(http.StatusOK, "hello world")
104+
})
105+
router.Run()
106+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
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` |
2+
| CorsGin.go:98:21:98:55 | slice literal | access-control-allow-origin header is set to `null`, and access-control-allow-credentials is set to `true` |
13
| 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` |
24
| 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` |
35
| 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` |

0 commit comments

Comments
 (0)