Add changes

github · Kwstubbs · Oct 18, 2023 · Oct 18, 2023 · Oct 18, 2023 · Oct 18, 2023
commit a4f32dc15eca750ab4b4696d52b1ece81d2f12ed
@@ -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.
@@ -0,0 +1,31 @@
+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
+}
@@ -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>
@@ -0,0 +1,51 @@
+/**
+ * @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
+
+class SafeJwtParserFunc extends Function {
+  SafeJwtParserFunc() {
+    this.hasQualifiedName(golangJwtPackage(), ["Parse", "ParseWithClaims"])
+  }
+}
+class SafeJwtParserMethod extends Method {
+  SafeJwtParserMethod() {
+    this.hasQualifiedName(golangJwtPackage(), "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.getAResult(), 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(golangJwtPackage(), "Token", "Method") and
+       f.getARead().getRoot() = c.getCall().getAnArgument()
+     )
+   )
+ select c, "This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion"
@@ -89,20 +89,52 @@ class GolangJwtParse extends JwtParseWithKeyFunction {
   override int getTokenArgNum() { result = 0 }
 }
 
+/**
+ * A class that parses JWT tokens while also extracting claims.
+ */
+abstract class GolangJwtParseWithClaims extends JwtParseWithKeyFunction {
+  GolangJwtParseWithClaims() {
+    exists(Function f | f.hasQualifiedName(golangJwtPackage(), "ParseWithClaims") | this = f)
+    or
+    exists(Method f | f.hasQualifiedName(golangJwtPackage(), "Parser", "ParseWithClaims") |
+      this = f
+    )
+  }
+
+  override int getKeyFuncArgNum() { result = 2 }
+
+  override int getTokenArgNum() { result = 0 }
+}
+
 /**
  * A class that contains the following function and method:
  *
  * func (p *Parser) ParseWithClaims(tokenString string, claims Claims, keyFunc Keyfunc)
+ */
+ class GolangJwtMethodParseWithClaims extends JwtParseWithKeyFunction {
+  GolangJwtMethodParseWithClaims() {
+    exists(Method f | f.hasQualifiedName(golangJwtPackage(), "Parser", "ParseWithClaims") |
+      this = f
+    )
+  }
+
+  override int getKeyFuncArgNum() { result = 2 }
+
+  override int getTokenArgNum() { result = 0 }
+}
+
+/**
+ * A class that contains the following function and method:
  *
  * func ParseWithClaims(tokenString string, claims Claims, keyFunc Keyfunc)
  */
-class GolangJwtParseWithClaims extends JwtParseWithKeyFunction {
-  GolangJwtParseWithClaims() {
-    exists(Function f | f.hasQualifiedName(golangJwtPackage(), "ParseWithClaims") | this = f)
-    or
-    exists(Method f | f.hasQualifiedName(golangJwtPackage(), "Parser", "ParseWithClaims") |
+
+class GolangJwtFunctionParseWithClaims extends JwtParseWithKeyFunction {
+  GolangJwtFunctionParseWithClaims() {
+    exists(Function f | f.hasQualifiedName(golangJwtPackage(), "ParseWithClaims") |
       this = f
     )
+
   }
 
   override int getKeyFuncArgNum() { result = 2 }
@@ -207,6 +239,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(golangJwtRequestPackage(), "WithValidMethods") }
+}
+
+/**
+ * A function in golang-jwt to create new parser.
+ */
+class NewParser extends Function {
+  NewParser() { this.hasQualifiedName(golangJwtRequestPackage(), "NewParser") }
+}
+
 /**
  * Holds for general additional steps related to parsing the secret keys in `golang-jwt/jwt`,`dgrijalva/jwt-go` packages
  */

@@ -0,0 +1,2 @@
+| JWTParsingAlgorithm.go:20:4:23:5 | call to Parse | This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion |
+| JWTParsingAlgorithm.go:38:22:41:5 | call to Parse | This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion |
@@ -0,0 +1,69 @@
+package main
+
+import (
+	"net/http"
+
+	"github.com/golang-jwt/jwt/v5"
+)
+
+func verifyJWT(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", func(token *jwt.Token) (interface{}, error) {
+				return "", nil
+
+			})
+			//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
+			}
+		}
+	})
+}
@@ -0,0 +1 @@
+experimental/CWE-347/JWTParsingAlgorithm.ql
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		\| JWTParsingAlgorithm.go:20:4:23:5 \| call to Parse \| This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion \|
		\| JWTParsingAlgorithm.go:38:22:41:5 \| call to Parse \| This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion \|