Go: Add JWT Algorithm Confusion and JWT decoding without Signature Verification #14081
Conversation
|
QHelp previews: go/ql/src/experimental/CWE-347/JWTParsing.qhelperrors/warnings: |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
@Kwstubbs why there is a lot of files that changed in my and your query is like this? |
|
@amammad We will have to reduce the size of our vendor folders with https://github.com/github/depstubber, just having a little trouble on my end getting it to work so I opened the PR first. The all for one if up to you, our PRs seems to have some overlap, but are not exactly the same. If you have find a CVE that is a signature verification issue and is modeled by your PR, feel free to open a bounty. |
|
QHelp previews: go/ql/src/experimental/CWE-347/JWTParsingAlgorithm.qhelpJWT Method CheckNot verifying the JWT algorithim present in a JWT token when verifying its signature may result in algorithim confusion. RecommendationBefore presenting a key to verify a signature, ensure that the key corresponds to the algorithim present in the JWT token. ExampleThe following code uses the asymmetric public key to verify the JWT token and assumes that the algorithim is RSA. By not checking the signature, an attacker can specify the HMAC option and use the same public key, which is assumed to be public and often at endpoint /jwt/jwks.jso, to sign the JWT token and bypass authentication. package main
import (
"fmt"
"github.com/golang-jwt/jwt/v5"
)
type JWT struct {
privateKey []byte
publicKey []byte
}
func (j JWT) Validate(token string) (interface{}, error) {
key, err := jwt.ParseRSAPublicKeyFromPEM(j.publicKey)
if err != nil {
return "", fmt.Errorf("validate: parse key: %w", err)
}
tok, err := jwt.Parse(token, func(jwtToken *jwt.Token) (interface{}, error) {
return key, nil
})
if err != nil {
return nil, fmt.Errorf("validate: %w", err)
}
claims, ok := tok.Claims.(jwt.MapClaims)
if !ok || !tok.Valid {
return nil, fmt.Errorf("validate: invalid")
}
return claims["dat"], nil
}References
|
|
@Kwstubbs could you address the code scanning findings flagged above and the test failures? |
| /** | ||
| * A v2 Function to parse token that check token signature. | ||
| */ | ||
| class LestrratParse extends Function { | ||
| LestrratParse() { this.hasQualifiedName(packageLestrrat(), "Parse") } | ||
| } |
There was a problem hiding this comment.
This model isn't used anywhere. Did you mean to use it somewhere? Or did you model it for completeness? Consider deleting - the model can easily be added in future if it is needed.
|
The requirement for a review from |
|
@Kwstubbs Do you think you'll get to this at some point? |
|
@owen-mc It seems that @amammad 's changes create a lot of conflicts. I will close this PR and open two new ones (in couple days), one focusing on the algorithm confusion and the other focusing on the additional lestrrat sinks for the "no signature verification" methods, that way things are simpler and easier to merge/understand. |
This PR adds support for detecting JWT go algorithm confusion vulnerability and decoding without signature verification. After opening this pull request, I noticed that my PR shares some similarity to this PR. This PR models some of lestrrat-go and the default go library