Go: Add JWT Algorithm Confusion Query

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.

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

go/ql/src/experimental/CWE-347/JWTParsingAlgorithm.qhelp

@@ -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>

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"

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
  */

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 |

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

go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.qlref

@@ -0,0 +1 @@
+experimental/CWE-347/JWTParsingAlgorithm.ql

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
 )