diff --git a/go/ql/src/change-notes/2023-10-17-algorithm-confusion-JWT-query.md b/go/ql/src/change-notes/2023-10-17-algorithm-confusion-JWT-query.md new file mode 100644 index 000000000000..d03f69873b1d --- /dev/null +++ b/go/ql/src/change-notes/2023-10-17-algorithm-confusion-JWT-query.md @@ -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. \ No newline at end of file diff --git a/go/ql/src/experimental/CWE-347/Algorithm.go b/go/ql/src/experimental/CWE-347/Algorithm.go new file mode 100644 index 000000000000..631549f56b55 --- /dev/null +++ b/go/ql/src/experimental/CWE-347/Algorithm.go @@ -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 +} diff --git a/go/ql/src/experimental/CWE-347/JWTParsingAlgorithm.qhelp b/go/ql/src/experimental/CWE-347/JWTParsingAlgorithm.qhelp new file mode 100644 index 000000000000..86b10d3f51e2 --- /dev/null +++ b/go/ql/src/experimental/CWE-347/JWTParsingAlgorithm.qhelp @@ -0,0 +1,35 @@ + + + +

+ Not verifying the JWT algorithim present in a JWT token when verifying its signature may result in algorithim confusion. +

+ +
+ + +

+ Before presenting a key to verify a signature, ensure that the key corresponds to the algorithim present in the JWT token. +

+ +
+ + +

+ 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. +

+ + + +
+ + +
  • Pentesterlab: Exploring Algorithm Confusion Attacks on JWT. +
  • +
    + +
    \ No newline at end of file diff --git a/go/ql/src/experimental/CWE-347/JWTParsingAlgorithm.ql b/go/ql/src/experimental/CWE-347/JWTParsingAlgorithm.ql new file mode 100644 index 000000000000..9542ad91c57a --- /dev/null +++ b/go/ql/src/experimental/CWE-347/JWTParsingAlgorithm.ql @@ -0,0 +1,64 @@ +/** + * @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, int i | + c2.getTarget() = m and + ( + c2.getSyntacticArgument(i) = wvm.getACall() and + DataFlow::localFlow(c2.getResult(0), c.getReceiver()) + ) + ) + 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 | + f.hasQualifiedName(golangJwtModern(), "Token", "Method") and + ( + f.getARead().getRoot() = c.getCall().getArgument(1) + or + exists(FunctionName fn | + 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" diff --git a/go/ql/src/experimental/frameworks/JWT.qll b/go/ql/src/experimental/frameworks/JWT.qll index ceb7ffc94099..55d7624d50c8 100644 --- a/go/ql/src/experimental/frameworks/JWT.qll +++ b/go/ql/src/experimental/frameworks/JWT.qll @@ -70,6 +70,13 @@ string golangJwtPackage() { result = package(["github.com/golang-jwt/jwt", "github.com/dgrijalva/jwt-go"], "") } +/** + * Gets `github.com/golang-jwt/jwt/(v4 and v5)` whose APIs have changed from previous versions. + */ +string golangJwtModern() { + result = ["github.com/golang-jwt/jwt/v5", "github.com/golang-jwt/jwt/v4"] +} + /** * A class that contains the following function and method: * @@ -207,6 +214,20 @@ class GoJoseUnsafeClaims extends JwtUnverifiedParse { override int getTokenArgNum() { result = -1 } } +/** + * A function in golang-jwt to specify allowed algorithms. + */ +class WithValidMethods extends Function { + WithValidMethods() { this.hasQualifiedName(golangJwtModern(), "WithValidMethods") } +} + +/** + * A function in golang-jwt to create new parser. + */ +class NewParser extends Function { + NewParser() { this.hasQualifiedName(golangJwtModern(), "NewParser") } +} + /** * Holds for general additional steps related to parsing the secret keys in `golang-jwt/jwt`,`dgrijalva/jwt-go` packages */ diff --git a/go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.expected b/go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.expected new file mode 100644 index 000000000000..9d54359d2bdf --- /dev/null +++ b/go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.expected @@ -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 | \ No newline at end of file diff --git a/go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.go b/go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.go new file mode 100644 index 000000000000..18235d7bb8b7 --- /dev/null +++ b/go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.go @@ -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 +} diff --git a/go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.qlref b/go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.qlref new file mode 100644 index 000000000000..25948a086af9 --- /dev/null +++ b/go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.qlref @@ -0,0 +1 @@ +experimental/CWE-347/JWTParsingAlgorithm.ql \ No newline at end of file diff --git a/go/ql/test/experimental/CWE-347/go.mod b/go/ql/test/experimental/CWE-347/go.mod index 84e592efcf35..861571c98ce2 100644 --- a/go/ql/test/experimental/CWE-347/go.mod +++ b/go/ql/test/experimental/CWE-347/go.mod @@ -11,27 +11,55 @@ require ( require ( github.com/bytedance/sonic v1.9.1 // indirect github.com/chenzhuoyu/base64x v0.0.0-20221115062448-fe3a3abad311 // indirect + github.com/dgryski/go-minhash v0.0.0-20170608043002-7fe510aff544 // indirect + github.com/ekzhu/minhash-lsh v0.0.0-20171225071031-5c06ee8586a1 // indirect + github.com/emirpasic/gods v1.12.0 // indirect github.com/gabriel-vasile/mimetype v1.4.2 // indirect github.com/gin-contrib/sse v0.1.0 // indirect + github.com/github/depstubber v0.0.0-20211124194836-d0e8ca3d2e44 // indirect + github.com/go-enry/go-license-detector/v4 v4.0.0 // indirect + github.com/go-git/gcfg v1.5.0 // indirect + github.com/go-git/go-billy/v5 v5.0.0 // indirect + github.com/go-git/go-git/v5 v5.1.0 // indirect github.com/go-playground/locales v0.14.1 // indirect github.com/go-playground/universal-translator v0.18.1 // indirect github.com/go-playground/validator/v10 v10.14.0 // indirect github.com/goccy/go-json v0.10.2 // indirect + github.com/golang/dep v0.5.4 // indirect + github.com/google/go-cmp v0.5.9 // indirect + github.com/hhatto/gorst v0.0.0-20181029133204-ca9f730cac5b // indirect + github.com/imdario/mergo v0.3.9 // indirect + github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect + github.com/jdkato/prose v1.1.0 // indirect github.com/json-iterator/go v1.1.12 // indirect + github.com/kevinburke/ssh_config v0.0.0-20190725054713-01f96b0aa0cd // indirect github.com/klauspost/cpuid/v2 v2.2.4 // indirect github.com/leodido/go-urn v1.2.4 // indirect github.com/mattn/go-isatty v0.0.19 // indirect + github.com/mitchellh/go-homedir v1.1.0 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect + github.com/montanaflynn/stats v0.0.0-20151014174947-eeaced052adb // indirect github.com/pelletier/go-toml/v2 v2.0.8 // indirect + github.com/pkg/errors v0.9.1 // indirect + github.com/russross/blackfriday/v2 v2.0.1 // indirect + github.com/sergi/go-diff v1.1.0 // indirect + github.com/shogo82148/go-shuffle v0.0.0-20170808115208-59829097ff3b // indirect + github.com/shurcooL/sanitized_anchor_name v0.0.0-20170918181015-86672fcb3f95 // indirect github.com/twitchyliquid64/golang-asm v0.15.1 // indirect github.com/ugorji/go/codec v1.2.11 // indirect + github.com/xanzy/ssh-agent v0.2.1 // indirect golang.org/x/arch v0.3.0 // indirect + golang.org/x/crypto v0.12.0 // indirect + golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2 // indirect + golang.org/x/mod v0.8.0 // indirect golang.org/x/net v0.10.0 // indirect golang.org/x/sys v0.11.0 // indirect golang.org/x/text v0.12.0 // indirect + golang.org/x/tools v0.6.0 // indirect + gonum.org/v1/gonum v0.7.0 // indirect google.golang.org/protobuf v1.30.0 // indirect + gopkg.in/neurosnap/sentences.v1 v1.0.6 // indirect + gopkg.in/warnings.v0 v0.1.2 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - github.com/google/go-cmp v0.5.9 // indirect - golang.org/x/crypto v0.12.0 // indirect ) diff --git a/go/ql/test/experimental/CWE-347/vendor/github.com/golang-jwt/jwt/v5/stub.go b/go/ql/test/experimental/CWE-347/vendor/github.com/golang-jwt/jwt/v5/stub.go index 6e4c4f327afa..2879db585366 100644 --- a/go/ql/test/experimental/CWE-347/vendor/github.com/golang-jwt/jwt/v5/stub.go +++ b/go/ql/test/experimental/CWE-347/vendor/github.com/golang-jwt/jwt/v5/stub.go @@ -2,12 +2,13 @@ // This is a simple stub for github.com/golang-jwt/jwt/v5, strictly for use in testing. // See the LICENSE file for information about the licensing of the original library. -// Source: github.com/golang-jwt/jwt/v5 (exports: RegisteredClaims,Parser,Token; functions: ParseWithClaims,NewParser) +// Source: github.com/golang-jwt/jwt/v5 (exports: RegisteredClaims,Parser,Token; functions: Parse,ParseWithClaims,NewParser,WithValidMethods) // Package jwt is a stub of github.com/golang-jwt/jwt/v5, generated by depstubber. package jwt import ( + crypto "crypto" time "time" ) @@ -220,6 +221,10 @@ func (_ *NumericDate) UnmarshalText(_ []byte) error { return nil } +func Parse(_ string, _ Keyfunc, _ ...ParserOption) (*Token, error) { + return nil, nil +} + func ParseWithClaims(_ string, _ Claims, _ Keyfunc, _ ...ParserOption) (*Token, error) { return nil, nil } @@ -304,3 +309,24 @@ func (_ *Token) SignedString(_ interface{}) (string, error) { func (_ *Token) SigningString() (string, error) { return "", nil } + +func WithValidMethods(_ []string) ParserOption { + return nil +} + +type SigningMethodHMAC struct { + Name string + Hash crypto.Hash +} + +func (_ *SigningMethodHMAC) Alg() string { + return "" +} + +func (_ *SigningMethodHMAC) Sign(_ string, _ interface{}) ([]byte, error) { + return []byte{}, nil +} + +func (_ *SigningMethodHMAC) Verify(_ string, _ []byte, _ interface{}) error { + return nil +} diff --git a/go/ql/test/experimental/CWE-347/vendor/modules.txt b/go/ql/test/experimental/CWE-347/vendor/modules.txt index 01144bc92497..20aae3e80107 100644 --- a/go/ql/test/experimental/CWE-347/vendor/modules.txt +++ b/go/ql/test/experimental/CWE-347/vendor/modules.txt @@ -13,12 +13,36 @@ github.com/bytedance/sonic # github.com/chenzhuoyu/base64x v0.0.0-20221115062448-fe3a3abad311 ## explicit github.com/chenzhuoyu/base64x +# github.com/dgryski/go-minhash v0.0.0-20170608043002-7fe510aff544 +## explicit +github.com/dgryski/go-minhash +# github.com/ekzhu/minhash-lsh v0.0.0-20171225071031-5c06ee8586a1 +## explicit +github.com/ekzhu/minhash-lsh +# github.com/emirpasic/gods v1.12.0 +## explicit +github.com/emirpasic/gods # github.com/gabriel-vasile/mimetype v1.4.2 ## explicit github.com/gabriel-vasile/mimetype # github.com/gin-contrib/sse v0.1.0 ## explicit github.com/gin-contrib/sse +# github.com/github/depstubber v0.0.0-20211124194836-d0e8ca3d2e44 +## explicit +github.com/github/depstubber +# github.com/go-enry/go-license-detector/v4 v4.0.0 +## explicit +github.com/go-enry/go-license-detector/v4 +# github.com/go-git/gcfg v1.5.0 +## explicit +github.com/go-git/gcfg +# github.com/go-git/go-billy/v5 v5.0.0 +## explicit +github.com/go-git/go-billy/v5 +# github.com/go-git/go-git/v5 v5.1.0 +## explicit +github.com/go-git/go-git/v5 # github.com/go-playground/locales v0.14.1 ## explicit github.com/go-playground/locales @@ -31,9 +55,30 @@ github.com/go-playground/validator/v10 # github.com/goccy/go-json v0.10.2 ## explicit github.com/goccy/go-json +# github.com/golang/dep v0.5.4 +## explicit +github.com/golang/dep +# github.com/google/go-cmp v0.5.9 +## explicit +github.com/google/go-cmp +# github.com/hhatto/gorst v0.0.0-20181029133204-ca9f730cac5b +## explicit +github.com/hhatto/gorst +# github.com/imdario/mergo v0.3.9 +## explicit +github.com/imdario/mergo +# github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 +## explicit +github.com/jbenet/go-context +# github.com/jdkato/prose v1.1.0 +## explicit +github.com/jdkato/prose # github.com/json-iterator/go v1.1.12 ## explicit github.com/json-iterator/go +# github.com/kevinburke/ssh_config v0.0.0-20190725054713-01f96b0aa0cd +## explicit +github.com/kevinburke/ssh_config # github.com/klauspost/cpuid/v2 v2.2.4 ## explicit github.com/klauspost/cpuid/v2 @@ -43,24 +88,57 @@ github.com/leodido/go-urn # github.com/mattn/go-isatty v0.0.19 ## explicit github.com/mattn/go-isatty +# github.com/mitchellh/go-homedir v1.1.0 +## explicit +github.com/mitchellh/go-homedir # github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd ## explicit github.com/modern-go/concurrent # github.com/modern-go/reflect2 v1.0.2 ## explicit github.com/modern-go/reflect2 +# github.com/montanaflynn/stats v0.0.0-20151014174947-eeaced052adb +## explicit +github.com/montanaflynn/stats # github.com/pelletier/go-toml/v2 v2.0.8 ## explicit github.com/pelletier/go-toml/v2 +# github.com/pkg/errors v0.9.1 +## explicit +github.com/pkg/errors +# github.com/russross/blackfriday/v2 v2.0.1 +## explicit +github.com/russross/blackfriday/v2 +# github.com/sergi/go-diff v1.1.0 +## explicit +github.com/sergi/go-diff +# github.com/shogo82148/go-shuffle v0.0.0-20170808115208-59829097ff3b +## explicit +github.com/shogo82148/go-shuffle +# github.com/shurcooL/sanitized_anchor_name v0.0.0-20170918181015-86672fcb3f95 +## explicit +github.com/shurcooL/sanitized_anchor_name # github.com/twitchyliquid64/golang-asm v0.15.1 ## explicit github.com/twitchyliquid64/golang-asm # github.com/ugorji/go/codec v1.2.11 ## explicit github.com/ugorji/go/codec +# github.com/xanzy/ssh-agent v0.2.1 +## explicit +github.com/xanzy/ssh-agent # golang.org/x/arch v0.3.0 ## explicit golang.org/x/arch +# golang.org/x/crypto v0.12.0 +## explicit +golang.org/x/crypto +# golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2 +## explicit +golang.org/x/exp +# golang.org/x/mod v0.8.0 +## explicit +golang.org/x/mod # golang.org/x/net v0.10.0 ## explicit golang.org/x/net @@ -70,15 +148,21 @@ golang.org/x/sys # golang.org/x/text v0.12.0 ## explicit golang.org/x/text +# golang.org/x/tools v0.6.0 +## explicit +golang.org/x/tools +# gonum.org/v1/gonum v0.7.0 +## explicit +gonum.org/v1/gonum # google.golang.org/protobuf v1.30.0 ## explicit google.golang.org/protobuf -# gopkg.in/yaml.v3 v3.0.1 +# gopkg.in/neurosnap/sentences.v1 v1.0.6 ## explicit -gopkg.in/yaml.v3 -# github.com/google/go-cmp v0.5.9 +gopkg.in/neurosnap/sentences.v1 +# gopkg.in/warnings.v0 v0.1.2 ## explicit -github.com/google/go-cmp -# golang.org/x/crypto v0.12.0 +gopkg.in/warnings.v0 +# gopkg.in/yaml.v3 v3.0.1 ## explicit -golang.org/x/crypto +gopkg.in/yaml.v3