Go: Add JWT Algorithm Confusion Query #14534
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| --- | ||
| category: newQuery | ||
| --- | ||
| * Added JWT Confusion query `go/jwt-alg-confusion`, a vulnerability where the application trusts the JWT token's header algorithm, allowing an application expecting to use asymmetric signature validation use symmetric signature, and thus bypassing JWT signature verification in cases where the public key is exposed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| 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 | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| <!DOCTYPE qhelp PUBLIC | ||
| "-//Semmle//qhelp//EN" | ||
| "qhelp.dtd"> | ||
| <qhelp> | ||
| <overview> | ||
| <p> | ||
| Not verifying the JWT algorithim present in a JWT token when verifying its signature may result in algorithim confusion. | ||
| </p> | ||
|
|
||
| </overview> | ||
| <recommendation> | ||
|
|
||
| <p> | ||
| Before presenting a key to verify a signature, ensure that the key corresponds to the algorithim present in the JWT token. | ||
| </p> | ||
|
|
||
| </recommendation> | ||
| <example> | ||
|
|
||
| <p> | ||
| The 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. | ||
| </p> | ||
|
|
||
| <sample src="Algorithm.go" /> | ||
|
|
||
| </example> | ||
|
|
||
| <references> | ||
| <li>Pentesterlab: <a | ||
| href="https://blog.pentesterlab.com/exploring-algorithm-confusion-attacks-on-jwt-exploiting-ecdsa-23f7ff83390f">Exploring Algorithm Confusion Attacks on JWT</a>. | ||
| </li> | ||
| </references> | ||
|
|
||
| </qhelp> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,62 @@ | ||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * @name JWT Method Check | ||||||||||||||||||||||||||
| * @description Trusting the Method provided by the incoming JWT token may lead to an algorithim confusion | ||||||||||||||||||||||||||
| * @kind problem | ||||||||||||||||||||||||||
| * @problem.severity error | ||||||||||||||||||||||||||
| * @security-severity 8.0 | ||||||||||||||||||||||||||
| * @precision medium | ||||||||||||||||||||||||||
| * @id go/jwt-alg-confusion | ||||||||||||||||||||||||||
| * @tags security | ||||||||||||||||||||||||||
| * external/cwe/cwe-347 | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import go | ||||||||||||||||||||||||||
| import experimental.frameworks.JWT | ||||||||||||||||||||||||||
| import DataFlow | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * A parse function that verifies signature and accepts all methods. | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| class SafeJwtParserFunc extends Function { | ||||||||||||||||||||||||||
| SafeJwtParserFunc() { this.hasQualifiedName(golangJwtModern(), ["Parse", "ParseWithClaims"]) } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * A parse method that verifies signature. | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| class SafeJwtParserMethod extends Method { | ||||||||||||||||||||||||||
| SafeJwtParserMethod() { | ||||||||||||||||||||||||||
| this.hasQualifiedName(golangJwtModern(), "Parser", ["Parse", "ParseWithClaims"]) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from CallNode c, Function func | ||||||||||||||||||||||||||
| where | ||||||||||||||||||||||||||
| ( | ||||||||||||||||||||||||||
| c.getTarget() = func and | ||||||||||||||||||||||||||
| // //Flow from NewParser to Parse (check that this call to Parse does not use a Parser that sets Valid Methods) | ||||||||||||||||||||||||||
| ( | ||||||||||||||||||||||||||
| func instanceof SafeJwtParserMethod and | ||||||||||||||||||||||||||
| not exists(CallNode c2, WithValidMethods wvm, NewParser m | | ||||||||||||||||||||||||||
| c2.getTarget() = m and | ||||||||||||||||||||||||||
| ( | ||||||||||||||||||||||||||
| c2.getCall().getAnArgument() = wvm.getACall().asExpr() and | ||||||||||||||||||||||||||
| DataFlow::localFlow(c2.getResult(0), c.getReceiver()) | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|
Comment on lines
+40
to
+46
Contributor
There was a problem hiding this comment. No need for the brackets either.
Suggested change
|
||||||||||||||||||||||||||
| or | ||||||||||||||||||||||||||
| //ParserFunc creates a new default Parser on call that accepts all methods | ||||||||||||||||||||||||||
| func instanceof SafeJwtParserFunc | ||||||||||||||||||||||||||
| ) and | ||||||||||||||||||||||||||
| //Check that the Parse(function or method) does not check the Token Method field, which most likely is a check for method type | ||||||||||||||||||||||||||
| not exists(Field f, FunctionName fn | | ||||||||||||||||||||||||||
| f.hasQualifiedName(golangJwtModern(), "Token", "Method") and | ||||||||||||||||||||||||||
| ( | ||||||||||||||||||||||||||
| f.getARead().getRoot() = c.getCall().getArgument(1) | ||||||||||||||||||||||||||
| or | ||||||||||||||||||||||||||
| c.getCall().getArgument(1) = fn and | ||||||||||||||||||||||||||
| fn.toString() = f.getARead().asExpr().getEnclosingFunction().getName() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| select c, "This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion" | ||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| | JWTParsingAlgorithm.go:22:4:22:27 | call to Parse | This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion | | ||
| | JWTParsingAlgorithm.go:37:22:40:5 | call to Parse | This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion | | ||
| | golang-jwt-v5.go:48:23:48:84 | call to ParseWithClaims | This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| package jwt | ||
|
|
||
| //go:generate depstubber -vendor github.com/golang-jwt/jwt/v5 RegisteredClaims,Parser,Token,SigningMethodHMAC Parse,ParseWithClaims,NewParser,WithValidMethods | ||
|
|
||
| import ( | ||
| "net/http" | ||
|
|
||
| "github.com/golang-jwt/jwt/v5" | ||
| ) | ||
|
|
||
| func verify(endpointHandler func(writer http.ResponseWriter, request *http.Request)) http.HandlerFunc { | ||
| return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { | ||
| if request.Header["Token"] != nil { | ||
| //Good, this specifiies an algorithim in the parser, and therefore does not need to check the algorithim in the key function | ||
| p := jwt.NewParser(jwt.WithValidMethods([]string{"RSA256"})) | ||
| p.Parse("test", func(token *jwt.Token) (interface{}, error) { | ||
| return "", nil | ||
|
|
||
| }) | ||
| //Bad, this parses with a custom parser that does not specify the algorithim/method and does not check the token's algorithm | ||
| p_bad := jwt.NewParser() | ||
| p_bad.Parse("test", key) | ||
| //Good, this checks the token Method | ||
| token, err := jwt.Parse(request.Header["Token"][0], func(token *jwt.Token) (interface{}, error) { | ||
| _, ok := token.Method.(*jwt.SigningMethodHMAC) | ||
| if !ok { | ||
| writer.WriteHeader(http.StatusUnauthorized) | ||
| _, err := writer.Write([]byte("You're Unauthorized")) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
| return "", nil | ||
|
|
||
| }) | ||
| //Bad, this parses using the default parser without checking the token Method | ||
| token_bad, err := jwt.Parse(request.Header["Token"][0], func(token *jwt.Token) (interface{}, error) { | ||
| return "", nil | ||
|
|
||
| }) | ||
| // parsing errors result | ||
| if err != nil { | ||
| writer.WriteHeader(http.StatusUnauthorized) | ||
| _, err2 := writer.Write([]byte("You're Unauthorized due to error parsing the JWT")) | ||
| if err2 != nil { | ||
| return | ||
| } | ||
|
|
||
| } | ||
| // if there's a token | ||
| if token.Valid || token_bad.Valid { | ||
| endpointHandler(writer, request) | ||
| } else { | ||
| writer.WriteHeader(http.StatusUnauthorized) | ||
| _, err := writer.Write([]byte("You're Unauthorized due to invalid token")) | ||
| if err != nil { | ||
| return | ||
| } | ||
| } | ||
| } else { | ||
| writer.WriteHeader(http.StatusUnauthorized) | ||
| _, err := writer.Write([]byte("You're Unauthorized due to No token in the header")) | ||
| if err != nil { | ||
| return | ||
| } | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| func key(token *jwt.Token) (interface{}, error) { | ||
| return "", nil | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| experimental/CWE-347/JWTParsingAlgorithm.ql |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.