Skip to content

Commit 38b0ed8

Browse files
am0o0am0o0
committed
fix issues according to codereview
1 parent 2579791 commit 38b0ed8

File tree

7 files changed

+67
-19
lines changed

7 files changed

+67
-19
lines changed

go/ql/src/experimental/CWE-321-V2/Example.go renamed to go/ql/src/experimental/CWE-321-V2/ExampleBad.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ package main
33
import (
44
"fmt"
55
"log"
6+
7+
"github.com/go-jose/go-jose/v3/jwt"
68
)
79

810
var JwtKey = []byte("AllYourBase")
911

1012
func main() {
1113
// BAD: usage of a harcoded Key
12-
verifyJWT(token)
14+
verifyJWT(JWTFromUser)
1315
}
1416

1517
func LoadJwtKey(token *jwt.Token) (interface{}, error) {

‎go/ql/src/experimental/CWE-321-V2/HardCodedKeys.ql‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,26 +12,27 @@
1212
import go
1313
import experimental.frameworks.JWT
1414

15-
module JwtPaseWithConstantKeyConfig implements DataFlow::ConfigSig {
15+
module JwtParseWithConstantKeyConfig implements DataFlow::ConfigSig {
1616
predicate isSource(DataFlow::Node source) { source.asExpr() instanceof StringLit }
1717

1818
predicate isSink(DataFlow::Node sink) {
1919
// first part is the JWT Parsing Functions that get a func type as an argument
2020
// Find a node that has flow to a key Function argument
2121
// then find the first result node of this Function which is the secret key
2222
exists(FuncDef fd, DataFlow::Node n, DataFlow::ResultNode rn |
23-
GolangJwtKeyFunc::flow(n, _) and fd = n.asExpr()
24-
|
23+
GolangJwtKeyFunc::flow(n, _) and
24+
sink = rn and
25+
fd = n.asExpr() and
2526
rn.getRoot() = fd and
26-
rn.getIndex() = 0 and
27-
sink = rn
27+
rn.getIndex() = 0
2828
)
2929
or
30-
exists(Function fd, DataFlow::ResultNode rn | GolangJwtKeyFunc::flow(fd.getARead(), _) |
30+
exists(Function f, DataFlow::ResultNode rn |
31+
GolangJwtKeyFunc::flow(f.getARead(), _) and
3132
// sink is result of a method
3233
sink = rn and
3334
// the method is belong to a function in which is used as a JWT function key
34-
rn.getRoot() = fd.getFuncDecl() and
35+
rn.getRoot() = f.getFuncDecl() and
3536
rn.getIndex() = 0
3637
)
3738
or
@@ -52,13 +53,13 @@ module GolangJwtKeyFuncConfig implements DataFlow::ConfigSig {
5253
}
5354
}
5455

55-
module JwtPaseWithConstantKey = TaintTracking::Global<JwtPaseWithConstantKeyConfig>;
56+
module JwtParseWithConstantKey = TaintTracking::Global<JwtParseWithConstantKeyConfig>;
5657

5758
module GolangJwtKeyFunc = TaintTracking::Global<GolangJwtKeyFuncConfig>;
5859

59-
import JwtPaseWithConstantKey::PathGraph
60+
import JwtParseWithConstantKey::PathGraph
6061

61-
from JwtPaseWithConstantKey::PathNode source, JwtPaseWithConstantKey::PathNode sink
62-
where JwtPaseWithConstantKey::flowPath(source, sink)
62+
from JwtParseWithConstantKey::PathNode source, JwtParseWithConstantKey::PathNode sink
63+
where JwtParseWithConstantKey::flowPath(source, sink)
6364
select sink.getNode(), source, sink, "This $@.", source.getNode(),
6465
"Constant Key is used as JWT Secret key"

‎go/ql/src/experimental/CWE-321-V2/HardcodedKeys.qhelp‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
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 parsing 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 cryptographically secure secret key during application initialization and using this generated key for future JWT parsing 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 parsing user provided JWTs. In this case, an attacker can very easily forge a token by using the hard-coded secret.
29+
</p>
30+
31+
<sample src="ExampleGood.go" />
32+
33+
</example>
34+
<references>
35+
<li>
36+
CVE-2022-0664:
37+
<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>
38+
</li>
39+
</references>
40+
41+
</qhelp>

‎go/ql/src/experimental/CWE-347/Example.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ package main
33
import (
44
"fmt"
55
"log"
6+
7+
"github.com/golang-jwt/jwt/v5"
68
)
79

810
func main() {
911
// BAD: only decode jwt without verification
1012
notVerifyJWT(token)
1113

12-
// GOOD: decode with verification or verifiy plus decode
14+
// GOOD: decode with verification or verify plus decode
1315
notVerifyJWT(token)
1416
VerifyJWT(token)
1517
}

‎go/ql/src/experimental/CWE-347/ParseJWTWithoutVerification.ql‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ module WithValidationConfig implements DataFlow::ConfigSig {
1616
predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
1717

1818
predicate isSink(DataFlow::Node sink) {
19-
sink = any(JwtParse parseUnverified).getTokenArg() or
20-
sink = any(JwtParseWithKeyFunction parseUnverified).getTokenArg()
19+
sink = any(JwtParse jwtParse).getTokenArg() or
20+
sink = any(JwtParseWithKeyFunction jwtParseWithKeyFunction).getTokenArg()
2121
}
2222

2323
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
@@ -52,4 +52,5 @@ import NoValidation::PathGraph
5252

5353
from NoValidation::PathNode source, NoValidation::PathNode sink
5454
where NoValidation::flowPath(source, sink)
55-
select sink.getNode(), source, sink, "This $@.", source.getNode(), "decode"
55+
select sink.getNode(), source, sink, "This JWT is parsed without verification and received from $@.",
56+
source.getNode(), "this user-controlled source"

‎go/ql/src/experimental/frameworks/JWT.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ class GoJoseUnsafeClaims extends JwtUnverifiedParse {
208208
}
209209

210210
/**
211-
* Holds for general additioanl steps related to parsing the secret keys in `golang-jwt/jwt`,`dgrijalva/jwt-go` packages
211+
* Holds for general additional steps related to parsing the secret keys in `golang-jwt/jwt`,`dgrijalva/jwt-go` packages
212212
*/
213213
predicate golangJwtIsAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
214214
exists(Function f, DataFlow::CallNode call |

‎go/ql/test/experimental/CWE-321-V2/go-jose.v3.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@ package jwt
44

55
import (
66
"fmt"
7-
"github.com/go-jose/go-jose/v3/jwt"
87
"net/http"
8+
9+
"github.com/go-jose/go-jose/v3/jwt"
910
)
1011

12+
// NOT OK
1113
var JwtKey = []byte("AllYourBase")
1214

1315
func main2(r *http.Request) {
14-
// NOT OK
1516
signedToken := r.URL.Query().Get("signedToken")
1617
verifyJWT(signedToken)
1718
}

0 commit comments

Comments
 (0)