Go: Improved JWT query, JWT decoding without verification #14075
Conversation
|
@github/codeql-go This PR is part of a bounty and is ready for review. If someone with write access could move this to ready for review I would appreciate it. Thanks. |
|
QHelp previews: go/ql/src/experimental/CWE-347/NoVerification.qhelpUse of JWT Methods that only decode user provided TokenA JSON Web Token (JWT) is used for authenticating and managing users in an application. Only Decoding JWTs without checking if they have a valid signature or not can lead to security vulnerabilities. RecommendationDon't use methods that only decode JWT, Instead use methods that verify the signature of JWT. ExampleThe following code you can see an Example from a popular Library. func main() {
// BAD: only decode jwt without verification
notVerifyJWT(token)
// GOOD: decode with verification or verifiy plus decode
notVerifyJWT(token)
VerifyJWT(token)
}
func notVerifyJWT(signedToken string) {
fmt.Println("only decoding JWT")
DecodedToken, _, err := jwt.NewParser().ParseUnverified(signedToken, &CustomerInfo{})
if claims, ok := DecodedToken.Claims.(*CustomerInfo); ok {
fmt.Printf("DecodedToken:%v\n", claims)
} else {
log.Fatal("error", err)
}
}
func LoadJwtKey(token *jwt.Token) (interface{}, error) {
return ARandomJwtKey, nil
}
func verifyJWT(signedToken string) {
fmt.Println("verifying JWT")
DecodedToken, err := jwt.ParseWithClaims(signedToken, &CustomerInfo{}, LoadJwtKey)
if claims, ok := DecodedToken.Claims.(*CustomerInfo); ok && DecodedToken.Valid {
fmt.Printf("NAME:%v ,ID:%v\n", claims.Name, claims.ID)
} else {
log.Fatal(err)
}
}References
|
![github-advanced-security[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
You're also failing a CI check with this message: |
|
And the tests you've added are all failing because they're missing the package clause at the top of the file. |
|
@owen-mc thank you so much for your comprehensive reviews, it made my job much easier to resolve the issues of my PRs. |
python/ql/lib/semmle/python/security/dataflow/PathInjectionQuery.qll
Outdated
Show resolved
Hide resolved
…ry.qll Co-authored-by: Rasmus Wriedt Larsen <rasmuswriedtlarsen@gmail.com>
|
QHelp previews: go/ql/src/experimental/CWE-347/ParseJWTWithoutVerification.qhelpUse of JWT Methods that only decode user provided TokenA JSON Web Token (JWT) is used for authenticating and managing users in an application. Only Decoding JWTs without checking if they have a valid signature or not can lead to security vulnerabilities. RecommendationDon't use methods that only decode JWT, Instead use methods that verify the signature of JWT. ExampleIn the following code you can see an Example from a popular Library. package main
import (
"fmt"
"log"
)
func main() {
// BAD: only decode jwt without verification
notVerifyJWT(token)
// GOOD: decode with verification or verifiy plus decode
notVerifyJWT(token)
VerifyJWT(token)
}
func notVerifyJWT(signedToken string) {
fmt.Println("only decoding JWT")
DecodedToken, _, err := jwt.NewParser().ParseUnverified(signedToken, &CustomerInfo{})
if claims, ok := DecodedToken.Claims.(*CustomerInfo); ok {
fmt.Printf("DecodedToken:%v\n", claims)
} else {
log.Fatal("error", err)
}
}
func LoadJwtKey(token *jwt.Token) (interface{}, error) {
return ARandomJwtKey, nil
}
func verifyJWT(signedToken string) {
fmt.Println("verifying JWT")
DecodedToken, err := jwt.ParseWithClaims(signedToken, &CustomerInfo{}, LoadJwtKey)
if claims, ok := DecodedToken.Claims.(*CustomerInfo); ok && DecodedToken.Valid {
fmt.Printf("NAME:%v ,ID:%v\n", claims.Name, claims.ID)
} else {
log.Fatal(err)
}
}References
|
|
Please rename |
| exists(FuncDef fd, DataFlow::Node n, DataFlow::ResultNode rn | | ||
| GolangJwtKeyFunc::flow(n, _) and fd = n.asExpr() | ||
| | | ||
| rn.getRoot() = fd and | ||
| rn.getIndex() = 0 and | ||
| sink = rn | ||
| ) | ||
| or | ||
| exists(Function fd, DataFlow::ResultNode rn | GolangJwtKeyFunc::flow(fd.getARead(), _) | | ||
| // sink is result of a method | ||
| sink = rn and | ||
| // the method is belong to a function in which is used as a JWT function key | ||
| rn.getRoot() = fd.getFuncDecl() and | ||
| rn.getIndex() = 0 | ||
| ) |
There was a problem hiding this comment.
These two exists have a huge amount in common. It would be better to combine them into one exists.
There was a problem hiding this comment.
I think If I merge them it can be much longer that it is.
|
QHelp previews: go/ql/src/experimental/CWE-321-V2/HardcodedKeys.qhelperrors/warnings:go/ql/src/experimental/CWE-347/ParseJWTWithoutVerification.qhelpUse of JWT Methods that only decode user provided TokenA JSON Web Token (JWT) is used for authenticating and managing users in an application. Only Decoding JWTs without checking if they have a valid signature or not can lead to security vulnerabilities. RecommendationDon't use methods that only decode JWT, Instead use methods that verify the signature of JWT. ExampleIn the following code you can see an Example from a popular Library. package main
import (
"fmt"
"log"
"github.com/golang-jwt/jwt/v5"
)
func main() {
// BAD: only decode jwt without verification
notVerifyJWT(token)
// GOOD: decode with verification or verify plus decode
notVerifyJWT(token)
VerifyJWT(token)
}
func notVerifyJWT(signedToken string) {
fmt.Println("only decoding JWT")
DecodedToken, _, err := jwt.NewParser().ParseUnverified(signedToken, &CustomerInfo{})
if claims, ok := DecodedToken.Claims.(*CustomerInfo); ok {
fmt.Printf("DecodedToken:%v\n", claims)
} else {
log.Fatal("error", err)
}
}
func LoadJwtKey(token *jwt.Token) (interface{}, error) {
return ARandomJwtKey, nil
}
func verifyJWT(signedToken string) {
fmt.Println("verifying JWT")
DecodedToken, err := jwt.ParseWithClaims(signedToken, &CustomerInfo{}, LoadJwtKey)
if claims, ok := DecodedToken.Claims.(*CustomerInfo); ok && DecodedToken.Valid {
fmt.Printf("NAME:%v ,ID:%v\n", claims.Name, claims.ID)
} else {
log.Fatal(err)
}
}References
|
|
QHelp previews: go/ql/src/experimental/CWE-321-V2/HardCodedKeys.qhelpDecoding JWT with hardcoded keyA JSON Web Token (JWT) is used for authenticating and managing users in an application. Using a hard-coded secret key for parsing JWT tokens in open source projects can leave the application using the token vulnerable to authentication bypasses. 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. 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. RecommendationGenerating a cryptographically secure secret key during application initialization and using this generated key for future JWT parsing requests can prevent this vulnerability. ExampleThe 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. package main
import (
"fmt"
"log"
"github.com/go-jose/go-jose/v3/jwt"
)
var JwtKey = []byte("AllYourBase")
func main() {
// BAD: usage of a harcoded Key
verifyJWT(JWTFromUser)
}
func LoadJwtKey(token *jwt.Token) (interface{}, error) {
return JwtKey, nil
}
func verifyJWT(signedToken string) {
fmt.Println("verifying JWT")
DecodedToken, err := jwt.ParseWithClaims(signedToken, &CustomerInfo{}, LoadJwtKey)
if claims, ok := DecodedToken.Claims.(*CustomerInfo); ok && DecodedToken.Valid {
fmt.Printf("NAME:%v ,ID:%v\n", claims.Name, claims.ID)
} else {
log.Fatal(err)
}
}References
go/ql/src/experimental/CWE-347/ParseJWTWithoutVerification.qhelpUse of JWT Methods that only decode user provided TokenA JSON Web Token (JWT) is used for authenticating and managing users in an application. Only Decoding JWTs without checking if they have a valid signature or not can lead to security vulnerabilities. RecommendationDon't use methods that only decode JWT, Instead use methods that verify the signature of JWT. ExampleIn the following code you can see an Example from a popular Library. package main
import (
"fmt"
"log"
"github.com/golang-jwt/jwt/v5"
)
func main() {
// BAD: only decode jwt without verification
notVerifyJWT(token)
// GOOD: decode with verification or verify plus decode
notVerifyJWT(token)
VerifyJWT(token)
}
func notVerifyJWT(signedToken string) {
fmt.Println("only decoding JWT")
DecodedToken, _, err := jwt.NewParser().ParseUnverified(signedToken, &CustomerInfo{})
if claims, ok := DecodedToken.Claims.(*CustomerInfo); ok {
fmt.Printf("DecodedToken:%v\n", claims)
} else {
log.Fatal("error", err)
}
}
func LoadJwtKey(token *jwt.Token) (interface{}, error) {
return ARandomJwtKey, nil
}
func verifyJWT(signedToken string) {
fmt.Println("verifying JWT")
DecodedToken, err := jwt.ParseWithClaims(signedToken, &CustomerInfo{}, LoadJwtKey)
if claims, ok := DecodedToken.Claims.(*CustomerInfo); ok && DecodedToken.Valid {
fmt.Printf("NAME:%v ,ID:%v\n", claims.Name, claims.ID)
} else {
log.Fatal(err)
}
}References
|
|
Two problems:
|
|
I made a suggestion to fix the formatting check and committed it. I didn't know I could do that. So you may have to pull to get that commit on your local machine. |
|
@owen-mc Thanks, I think when I opened the pull request I gave access to maintainers to perform their changes. |
|
@amammad One last thing - could you edit the description of this PR with a brief summary of it? |
|
@owen-mc Its done! |
Some functions/methods of some JWT packages do not verify the JWT signature, and sometimes developers use it by mistake, if developers are using this intentionally then they should be careful about future contributions so I think especially in open source projects this is not recommended to using this method.
Also, I wrote a query that improves the current hardcoded JWT secret keys query because Some JWT packages use a function as input to return the secret on some conditions that developers specify. I used two Taint module configurations one for finding these functions and two for any constant key that can be tainted to first return the value of these key functions. So with this, we can be sure that we can find the right constant secret key (if it exists). I found some results with MRVA which are new!