Skip to content

Commit 4e59ac4

Browse files
owen-mcowen-mc
authored
Merge pull request #14873 from Kwstubbs/go-rs-cors
Go: Add Rs Cors Support
2 parents 66777e6 + 217bc74 commit 4e59ac4

File tree

10 files changed

+302
-16
lines changed

10 files changed

+302
-16
lines changed

‎go/ql/lib/change-notes/2023-10-31-add-rs-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 [rs cors](https://github.com/rs/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
@@ -32,6 +32,7 @@ import semmle.go.frameworks.Afero
3232
import semmle.go.frameworks.AwsLambda
3333
import semmle.go.frameworks.Beego
3434
import semmle.go.frameworks.BeegoOrm
35+
import semmle.go.frameworks.RsCors
3536
import semmle.go.frameworks.Couchbase
3637
import semmle.go.frameworks.Echo
3738
import semmle.go.frameworks.ElazarlGoproxy

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ module GinCors {
2121
/**
2222
* A write to the value of Access-Control-Allow-Credentials header
2323
*/
24-
class AllowCredentialsWrite extends DataFlow::ExprNode {
24+
class AllowCredentialsWrite extends UniversalAllowCredentialsWrite {
2525
DataFlow::Node base;
2626

2727
AllowCredentialsWrite() {
@@ -35,12 +35,12 @@ module GinCors {
3535
/**
3636
* Get config struct holding header values
3737
*/
38-
DataFlow::Node getBase() { result = base }
38+
override DataFlow::Node getBase() { result = base }
3939

4040
/**
4141
* Get config variable holding header values
4242
*/
43-
GinConfig getConfig() {
43+
override GinConfig getConfig() {
4444
exists(GinConfig gc |
4545
(
4646
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
@@ -55,7 +55,7 @@ module GinCors {
5555
/**
5656
* A write to the value of Access-Control-Allow-Origins header
5757
*/
58-
class AllowOriginsWrite extends DataFlow::ExprNode {
58+
class AllowOriginsWrite extends UniversalOriginWrite {
5959
DataFlow::Node base;
6060

6161
AllowOriginsWrite() {
@@ -69,12 +69,12 @@ module GinCors {
6969
/**
7070
* Get config struct holding header values
7171
*/
72-
DataFlow::Node getBase() { result = base }
72+
override DataFlow::Node getBase() { result = base }
7373

7474
/**
7575
* Get config variable holding header values
7676
*/
77-
GinConfig getConfig() {
77+
override GinConfig getConfig() {
7878
exists(GinConfig gc |
7979
(
8080
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
@@ -89,7 +89,7 @@ module GinCors {
8989
/**
9090
* A write to the value of Access-Control-Allow-Origins of value "*", overriding AllowOrigins
9191
*/
92-
class AllowAllOriginsWrite extends DataFlow::ExprNode {
92+
class AllowAllOriginsWrite extends UniversalAllowAllOriginsWrite {
9393
DataFlow::Node base;
9494

9595
AllowAllOriginsWrite() {
@@ -103,12 +103,12 @@ module GinCors {
103103
/**
104104
* Get config struct holding header values
105105
*/
106-
DataFlow::Node getBase() { result = base }
106+
override DataFlow::Node getBase() { result = base }
107107

108108
/**
109109
* Get config variable holding header values
110110
*/
111-
GinConfig getConfig() {
111+
override GinConfig getConfig() {
112112
exists(GinConfig gc |
113113
(
114114
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =

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

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
/**
2+
* Provides classes for modeling the `github.com/rs/cors` package.
3+
*/
4+
5+
import go
6+
7+
/**
8+
* An abstract class for modeling the Go CORS handler model origin write.
9+
*/
10+
abstract class UniversalOriginWrite extends DataFlow::ExprNode {
11+
/**
12+
* Get config variable holding header values
13+
*/
14+
abstract DataFlow::Node getBase();
15+
16+
/**
17+
* Get config variable holding header values
18+
*/
19+
abstract Variable getConfig();
20+
}
21+
22+
/**
23+
* An abstract class for modeling the Go CORS handler model allow all origins write.
24+
*/
25+
abstract class UniversalAllowAllOriginsWrite extends DataFlow::ExprNode {
26+
/**
27+
* Get config variable holding header values
28+
*/
29+
abstract DataFlow::Node getBase();
30+
31+
/**
32+
* Get config variable holding header values
33+
*/
34+
abstract Variable getConfig();
35+
}
36+
37+
/**
38+
* An abstract class for modeling the Go CORS handler model allow credentials write.
39+
*/
40+
abstract class UniversalAllowCredentialsWrite extends DataFlow::ExprNode {
41+
/**
42+
* Get config struct holding header values
43+
*/
44+
abstract DataFlow::Node getBase();
45+
46+
/**
47+
* Get config variable holding header values
48+
*/
49+
abstract Variable getConfig();
50+
}
51+
52+
/**
53+
* Provides classes for modeling the `github.com/rs/cors` package.
54+
*/
55+
module RsCors {
56+
/** Gets the package name `github.com/gin-gonic/gin`. */
57+
string packagePath() { result = package("github.com/rs/cors", "") }
58+
59+
/**
60+
* A new function create a new rs Handler
61+
*/
62+
class New extends Function {
63+
New() { exists(Function f | f.hasQualifiedName(packagePath(), "New") | this = f) }
64+
}
65+
66+
/**
67+
* A write to the value of Access-Control-Allow-Credentials header
68+
*/
69+
class AllowCredentialsWrite extends UniversalAllowCredentialsWrite {
70+
DataFlow::Node base;
71+
72+
AllowCredentialsWrite() {
73+
exists(Field f, Write w |
74+
f.hasQualifiedName(packagePath(), "Options", "AllowCredentials") and
75+
w.writesField(base, f, this) and
76+
this.getType() instanceof BoolType
77+
)
78+
}
79+
80+
/**
81+
* Get options struct holding header values
82+
*/
83+
override DataFlow::Node getBase() { result = base }
84+
85+
/**
86+
* Get options variable holding header values
87+
*/
88+
override RsOptions getConfig() {
89+
exists(RsOptions gc |
90+
(
91+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
92+
base.asInstruction() or
93+
gc.getV().getAUse() = base
94+
) and
95+
result = gc
96+
)
97+
}
98+
}
99+
100+
/**
101+
* A write to the value of Access-Control-Allow-Origins header
102+
*/
103+
class AllowOriginsWrite extends UniversalOriginWrite {
104+
DataFlow::Node base;
105+
106+
AllowOriginsWrite() {
107+
exists(Field f, Write w |
108+
f.hasQualifiedName(packagePath(), "Options", "AllowedOrigins") and
109+
w.writesField(base, f, this) and
110+
this.asExpr() instanceof SliceLit
111+
)
112+
}
113+
114+
/**
115+
* Get options struct holding header values
116+
*/
117+
override DataFlow::Node getBase() { result = base }
118+
119+
/**
120+
* Get options variable holding header values
121+
*/
122+
override RsOptions getConfig() {
123+
exists(RsOptions gc |
124+
(
125+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
126+
base.asInstruction() or
127+
gc.getV().getAUse() = base
128+
) and
129+
result = gc
130+
)
131+
}
132+
}
133+
134+
/**
135+
* A write to the value of Access-Control-Allow-Origins of value "*", overriding AllowOrigins
136+
*/
137+
class AllowAllOriginsWrite extends UniversalAllowAllOriginsWrite {
138+
DataFlow::Node base;
139+
140+
AllowAllOriginsWrite() {
141+
exists(Field f, Write w |
142+
f.hasQualifiedName(packagePath(), "Options", "AllowAllOrigins") and
143+
w.writesField(base, f, this) and
144+
this.getType() instanceof BoolType
145+
)
146+
}
147+
148+
/**
149+
* Get options struct holding header values
150+
*/
151+
override DataFlow::Node getBase() { result = base }
152+
153+
/**
154+
* Get options variable holding header values
155+
*/
156+
override RsOptions getConfig() {
157+
exists(RsOptions gc |
158+
(
159+
gc.getV().getBaseVariable().getDefinition().(SsaExplicitDefinition).getRhs() =
160+
base.asInstruction() or
161+
gc.getV().getAUse() = base
162+
) and
163+
result = gc
164+
)
165+
}
166+
}
167+
168+
/**
169+
* A variable of type Options that holds the headers to be set.
170+
*/
171+
class RsOptions extends Variable {
172+
SsaWithFields v;
173+
174+
RsOptions() {
175+
this = v.getBaseVariable().getSourceVariable() and
176+
exists(Type t | t.hasQualifiedName(packagePath(), "Options") | v.getType() = t)
177+
}
178+
179+
/**
180+
* Get variable declaration of Options
181+
*/
182+
SsaWithFields getV() { result = v }
183+
}
184+
}

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

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

75-
additional predicate isSinkWrite(DataFlow::Node sink, GinCors::AllowOriginsWrite w) { sink = w }
75+
additional predicate isSinkWrite(DataFlow::Node sink, UniversalOriginWrite w) { sink = w }
7676

7777
predicate isSink(DataFlow::Node sink) { isSinkWrite(sink, _) }
7878
}
@@ -102,17 +102,17 @@ predicate allowCredentialsIsSetToTrue(DataFlow::ExprNode allowOriginHW) {
102102
allowCredentialsHW.getResponseWriter()
103103
)
104104
or
105-
exists(GinCors::AllowCredentialsWrite allowCredentialsGin |
105+
exists(UniversalAllowCredentialsWrite allowCredentialsGin |
106106
allowCredentialsGin.getExpr().getBoolValue() = true
107107
|
108-
allowCredentialsGin.getConfig() = allowOriginHW.(GinCors::AllowOriginsWrite).getConfig() and
109-
not exists(GinCors::AllowAllOriginsWrite allowAllOrigins |
108+
allowCredentialsGin.getConfig() = allowOriginHW.(UniversalOriginWrite).getConfig() and
109+
not exists(UniversalAllowAllOriginsWrite allowAllOrigins |
110110
allowAllOrigins.getExpr().getBoolValue() = true and
111111
allowCredentialsGin.getConfig() = allowAllOrigins.getConfig()
112112
)
113113
or
114-
allowCredentialsGin.getBase() = allowOriginHW.(GinCors::AllowOriginsWrite).getBase() and
115-
not exists(GinCors::AllowAllOriginsWrite allowAllOrigins |
114+
allowCredentialsGin.getBase() = allowOriginHW.(UniversalOriginWrite).getBase() and
115+
not exists(UniversalAllowAllOriginsWrite allowAllOrigins |
116116
allowAllOrigins.getExpr().getBoolValue() = true and
117117
allowCredentialsGin.getBase() = allowAllOrigins.getBase()
118118
)
@@ -150,7 +150,7 @@ predicate allowOriginIsNull(DataFlow::ExprNode allowOriginHW, string message) {
150150
+ " is set to `true`"
151151
or
152152
allowOriginHW
153-
.(GinCors::AllowOriginsWrite)
153+
.(UniversalOriginWrite)
154154
.asExpr()
155155
.(SliceLit)
156156
.getAnElement()

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@
55
| 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` |
66
| 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` |
77
| 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` |
8+
| 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` |

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package main
2+
3+
import (
4+
"net/http"
5+
6+
"github.com/rs/cors"
7+
)
8+
9+
func rs_vulnerable() {
10+
c := cors.New(cors.Options{
11+
AllowedOrigins: []string{"null", "http://foo.com:8080"},
12+
AllowCredentials: true,
13+
// Enable Debugging for testing, consider disabling in production
14+
Debug: true,
15+
})
16+
17+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
18+
w.Header().Set("Content-Type", "application/json")
19+
w.Write([]byte("{\"hello\": \"world\"}"))
20+
})
21+
22+
http.ListenAndServe(":8080", c.Handler(handler))
23+
}
24+
25+
func rs_safe() {
26+
c := cors.New(cors.Options{
27+
AllowedOrigins: []string{"http://foo.com:8080"},
28+
AllowCredentials: true,
29+
// Enable Debugging for testing, consider disabling in production
30+
Debug: true,
31+
})
32+
33+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
34+
w.Header().Set("Content-Type", "application/json")
35+
w.Write([]byte("{\"hello\": \"world\"}"))
36+
})
37+
38+
http.ListenAndServe(":8080", c.Handler(handler))
39+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ go 1.21
55
require (
66
github.com/gin-contrib/cors v1.4.0
77
github.com/gin-gonic/gin v1.9.1
8+
github.com/rs/cors v1.10.1
89
)
910

1011
require (

0 commit comments

Comments
 (0)