Skip to content

Commit bd1ddc1

Browse files
author
Porcupiney Hairs
committed
Golang : Add query to detect JWT signing vulnerabilities
Supersedes github/codeql-go#705
1 parent a661a0c commit bd1ddc1

File tree

20 files changed

+2379
-0
lines changed

20 files changed

+2379
-0
lines changed
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
A JSON Web Token (JWT) is used for authenticating and managing users in an application.
6+
</p>
7+
<p>
8+
Using a hard-coded secret key for signing JWT tokens in open source projects
9+
can leave the application using the token vulnerable to authentication bypasses.
10+
</p>
11+
12+
<p>
13+
A JWT token is safe for enforcing authentication and access control as long as it can't be forged by a malicious actor. However, when a project exposes this secret publicly, these seemingly unforgeable tokens can now be easily forged.
14+
Since the authentication as well as access control is typically enforced through these JWT tokens, an attacker armed with the secret can create a valid authentication token for any user and may even gain access to other privileged parts of the application.
15+
</p>
16+
17+
</overview>
18+
<recommendation>
19+
20+
<p>
21+
Generating a crytograhically secure secret key during application initialization and using this generated key for future JWT signing requests can prevent this vulnerability.
22+
</p>
23+
24+
</recommendation>
25+
<example>
26+
27+
<p>
28+
The following code uses a hard-coded string as a secret for signing the tokens. In this case, an attacker can very easily forge a token by using the hard-coded secret.
29+
</p>
30+
31+
<sample src="HardcodedKeysBad.go" />
32+
33+
</example>
34+
<example>
35+
36+
<p>
37+
In the following case, the application uses a programatically generated string as a secret for signing the tokens. In this case, since the secret can't be predicted, the code is secure. A function like `GenerateCryptoString` can be run to generate a secure secret key at the time of application installation/initialization. This generated key can then be used for all future signing requests.
38+
</p>
39+
40+
<sample src="HardcodedKeysGood.go" />
41+
42+
</example>
43+
<references>
44+
<li>
45+
CVE-2022-0664:
46+
<a href="https://nvd.nist.gov/vuln/detail/CVE-2022-0664">Use of Hard-coded Cryptographic Key in Go github.com/gravitl/netmaker prior to 0.8.5,0.9.4,0.10.0,0.10.1. </a>
47+
</li>
48+
</references>
49+
50+
</qhelp>
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* @name Use of a hardcoded key for signing JWT
3+
* @description Using a fixed hardcoded key for signing JWT's can allow an attacker to compromise security.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @id go/hardcoded-key
7+
* @tags security
8+
* external/cwe/cwe-321
9+
*/
10+
11+
import go
12+
import HardcodedKeysLib
13+
import DataFlow::PathGraph
14+
15+
from HardcodedKeys::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
16+
where cfg.hasFlowPath(source, sink)
17+
select sink.getNode(), source, sink, "$@ is used to sign a JWT token.", source.getNode(),
18+
"Hardcoded String"
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
mySigningKey := []byte("AllYourBase")
2+
3+
claims := &jwt.RegisteredClaims{
4+
ExpiresAt: jwt.NewNumericDate(time.Unix(1516239022, 0)),
5+
Issuer: "test",
6+
}
7+
8+
token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
9+
ss, err := token.SignedString(mySigningKey)
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
func GenerateCryptoString(n int) (string, error) {
2+
const chars = "123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-"
3+
ret := make([]byte, n)
4+
for i := range ret {
5+
num, err := crand.Int(crand.Reader, big.NewInt(int64(len(chars))))
6+
if err != nil {
7+
return "", err
8+
}
9+
ret[i] = chars[num.Int64()]
10+
}
11+
return string(ret), nil
12+
}
13+
14+
mySigningKey := GenerateCryptoString(64)
15+
16+
17+
claims := &jwt.RegisteredClaims{
18+
ExpiresAt: jwt.NewNumericDate(time.Unix(1516239022, 0)),
19+
Issuer: "test",
20+
}
21+
22+
token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
23+
ss, err := token.SignedString(mySigningKey)
Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for reasoning about
3+
* JWT token signing vulnerabilities as well as extension points
4+
* for adding your own.
5+
*/
6+
7+
import go
8+
import StringOps
9+
import DataFlow::PathGraph
10+
11+
/**
12+
* Provides default sources, sinks and sanitizers for reasoning about
13+
* JWT token signing vulnerabilities as well as extension points
14+
* for adding your own.
15+
*/
16+
module HardcodedKeys {
17+
/**
18+
* A data flow source for JWT token signing vulnerabilities.
19+
*/
20+
abstract class Source extends DataFlow::Node { }
21+
22+
/**
23+
* A data flow sink for JWT token signing vulnerabilities.
24+
*/
25+
abstract class Sink extends DataFlow::Node { }
26+
27+
/**
28+
* A sanitizer for JWT token signing vulnerabilities.
29+
*/
30+
abstract class Sanitizer extends DataFlow::Node { }
31+
32+
/**
33+
* A sanitizer guard for JWT token signing vulnerabilities.
34+
*/
35+
abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
36+
37+
private predicate isTestCode(Expr e) {
38+
e.getFile().getAbsolutePath().toLowerCase().matches("%test%") and
39+
not e.getFile().getAbsolutePath().toLowerCase().matches("%ql/test%")
40+
}
41+
42+
private predicate isDemoCode(Expr e) {
43+
e.getFile().getAbsolutePath().toLowerCase().matches(["%mock%", "%demo%", "%example%"])
44+
}
45+
46+
/**
47+
* A hardcoded string literal as a source for JWT token signing vulnerabilities.
48+
*/
49+
class HardcodedStringSource extends Source {
50+
HardcodedStringSource() {
51+
this.asExpr() instanceof StringLit and
52+
not (isTestCode(this.asExpr()) or isDemoCode(this.asExpr()))
53+
}
54+
}
55+
56+
/**
57+
* An expression used to sign JWT tokens as a sink for JWT token signing vulnerabilities.
58+
*/
59+
private class GolangJwtSign extends Sink {
60+
GolangJwtSign() {
61+
exists(string pkg |
62+
pkg =
63+
[
64+
"github.com/golang-jwt/jwt/v4", "github.com/dgrijalva/jwt-go",
65+
"github.com/form3tech-oss/jwt-go", "github.com/ory/fosite/token/jwt"
66+
]
67+
|
68+
(
69+
exists(DataFlow::MethodCallNode m |
70+
// Models the `SignedString` method
71+
// `func (t *Token) SignedString(key interface{}) (string, error)`
72+
m.getTarget().hasQualifiedName(pkg, "Token", "SignedString")
73+
|
74+
this = m.getArgument(0)
75+
)
76+
or
77+
exists(DataFlow::MethodCallNode m |
78+
// Model the `Sign` method of the `SigningMethod` interface
79+
// type SigningMethod interface {
80+
// Verify(signingString, signature string, key interface{}) error
81+
// Sign(signingString string, key interface{}) (string, error)
82+
// Alg() string
83+
// }
84+
m.getTarget().hasQualifiedName(pkg, "SigningMethod", "Sign")
85+
|
86+
this = m.getArgument(1)
87+
)
88+
)
89+
)
90+
}
91+
}
92+
93+
private class GinJwtSign extends Sink {
94+
GinJwtSign() {
95+
exists(Field f |
96+
// https://pkg.go.dev/github.com/appleboy/gin-jwt/v2#GinJWTMiddleware
97+
f.hasQualifiedName("github.com/appleboy/gin-jwt/v2", "GinJWTMiddleware", "Key") and
98+
f.getAWrite().getRhs() = this
99+
)
100+
}
101+
}
102+
103+
private class SquareJoseKey extends Sink {
104+
SquareJoseKey() {
105+
exists(Field f, string pkg |
106+
// type Recipient struct {
107+
// Algorithm KeyAlgorithm
108+
// Key interface{}
109+
// KeyID string
110+
// PBES2Count int
111+
// PBES2Salt []byte
112+
// }
113+
// type SigningKey struct {
114+
// Algorithm SignatureAlgorithm
115+
// Key interface{}
116+
// }
117+
f.hasQualifiedName(pkg, ["Recipient", "SigningKey"], "Key") and
118+
f.getAWrite().getRhs() = this
119+
|
120+
pkg = ["github.com/square/go-jose/v3", "gopkg.in/square/go-jose.v2"]
121+
)
122+
}
123+
}
124+
125+
private class CrystalHqJwtSigner extends Sink {
126+
CrystalHqJwtSigner() {
127+
exists(DataFlow::CallNode m |
128+
// `func NewSignerHS(alg Algorithm, key []byte) (Signer, error)`
129+
m.getTarget().hasQualifiedName("github.com/cristalhq/jwt/v3", "NewSignerHS")
130+
|
131+
this = m.getArgument(1)
132+
)
133+
}
134+
}
135+
136+
private class GoKitJwt extends Sink {
137+
GoKitJwt() {
138+
exists(DataFlow::CallNode m |
139+
// `func NewSigner(kid string, key []byte, method jwt.SigningMethod, claims jwt.Claims) endpoint.Middleware`
140+
m.getTarget().hasQualifiedName("github.com/go-kit/kit/auth/jwt", "NewSigner")
141+
|
142+
this = m.getArgument(1)
143+
)
144+
}
145+
}
146+
147+
private class LestrratJwk extends Sink {
148+
LestrratJwk() {
149+
exists(DataFlow::CallNode m, string pkg |
150+
pkg.matches([
151+
"github.com/lestrrat-go/jwx", "github.com/lestrrat/go-jwx/jwk",
152+
"github.com/lestrrat-go/jwx%/jwk"
153+
]) and
154+
// `func New(key interface{}) (Key, error)`
155+
m.getTarget().hasQualifiedName(pkg, "New")
156+
|
157+
this = m.getArgument(0)
158+
)
159+
}
160+
}
161+
162+
/**
163+
* Mark any comparision expression where any operand is tainted as a
164+
* sanitizer for all instances of the taint
165+
*/
166+
private class CompareExprSanitizer extends Sanitizer {
167+
CompareExprSanitizer() {
168+
exists(BinaryExpr c |
169+
c.getAnOperand().getGlobalValueNumber() = this.asExpr().getGlobalValueNumber()
170+
)
171+
}
172+
}
173+
174+
/** Mark an empty string returned with an error as a sanitizer */
175+
class EmptyErrorSanitizer extends Sanitizer {
176+
EmptyErrorSanitizer() {
177+
exists(ReturnStmt r, DataFlow::CallNode c |
178+
c.getTarget().hasQualifiedName("errors", "New") and
179+
r.getNumChild() > 1 and
180+
r.getAChild() = c.getAResult().getASuccessor*().asExpr() and
181+
r.getAChild() = this.asExpr()
182+
)
183+
}
184+
}
185+
186+
/** Mark any formatting string call as a sanitizer */
187+
class FormattingSanitizer extends Sanitizer {
188+
FormattingSanitizer() { exists(Formatting::StringFormatCall s | s.getAResult() = this) }
189+
}
190+
191+
/**
192+
* Mark any taint arising from a read on a tainted slice with a random index as a
193+
* sanitizer for all instances of the taint
194+
*/
195+
private class RandSliceSanitizer extends Sanitizer {
196+
RandSliceSanitizer() {
197+
exists(DataFlow::CallNode randint, string name, DataFlow::ElementReadNode r |
198+
(
199+
randint.getTarget().hasQualifiedName("math/rand", name) or
200+
randint.getTarget().(Method).hasQualifiedName("math/rand", "Rand", name)
201+
) and
202+
name =
203+
[
204+
"ExpFloat64", "Float32", "Float64", "Int", "Int31", "Int31n", "Int63", "Int63n", "Intn",
205+
"NormFloat64", "Uint32", "Uint64"
206+
] and
207+
r.reads(this, randint.getAResult().getASuccessor*())
208+
)
209+
or
210+
// Sanitize flows like this:
211+
// func GenerateCryptoString(n int) (string, error) {
212+
// const chars = "123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-"
213+
// ret := make([]byte, n)
214+
// for i := range ret {
215+
// num, err := crand.Int(crand.Reader, big.NewInt(int64(len(chars))))
216+
// if err != nil {
217+
// return "", err
218+
// }
219+
// ret[i] = chars[num.Int64()]
220+
// }
221+
// return string(ret), nil
222+
// }
223+
exists(
224+
DataFlow::CallNode randint, DataFlow::MethodCallNode bigint, DataFlow::ElementReadNode r
225+
|
226+
randint.getTarget().hasQualifiedName("crypto/rand", "Int") and
227+
bigint.getTarget().hasQualifiedName("math/big", "Int", "Int64") and
228+
bigint.getReceiver() = randint.getResult(0).getASuccessor*() and
229+
r.reads(this, bigint.getAResult().getASuccessor*())
230+
)
231+
or
232+
// Sanitize flows like :
233+
// func GenerateRandomString(size int) string {
234+
// var bytes = make([]byte, size)
235+
// rand.Read(bytes)
236+
// for i, x := range bytes {
237+
// bytes[i] = characters[x%byte(len(characters))]
238+
// }
239+
// return string(bytes)
240+
// }
241+
exists(DataFlow::CallNode randread, DataFlow::Node rand, DataFlow::ElementReadNode r |
242+
randread.getTarget().hasQualifiedName("crypto/rand", "Read") and
243+
TaintTracking::localTaint(randread.getArgument(0).getAPredecessor*().getASuccessor*(), rand) and
244+
(
245+
exists(ModExpr e | e.getAnOperand() = rand.asExpr() |
246+
r.reads(this, e.getGlobalValueNumber().getANode())
247+
)
248+
or
249+
r.reads(this.getAPredecessor*(), rand)
250+
)
251+
)
252+
}
253+
}
254+
255+
/**
256+
* A configuration depicting taint flow for studying JWT token signing vulnerabilities.
257+
*/
258+
class Configuration extends TaintTracking::Configuration {
259+
Configuration() { this = "Hard-coded JWT Signing Key" }
260+
261+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
262+
263+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
264+
265+
override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer instanceof Sanitizer }
266+
267+
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
268+
guard instanceof SanitizerGuard
269+
}
270+
}
271+
}

0 commit comments

Comments
 (0)