Go: Add JWT Algorithm Confusion and JWT decoding without Signature Verification

go/ql/lib/go.qll

@@ -43,6 +43,7 @@ import semmle.go.frameworks.Glog
 import semmle.go.frameworks.GoMicro
 import semmle.go.frameworks.GoRestfulHttp
 import semmle.go.frameworks.Gqlgen
+import semmle.go.frameworks.JWT
 import semmle.go.frameworks.K8sIoApimachineryPkgRuntime
 import semmle.go.frameworks.K8sIoApiCoreV1
 import semmle.go.frameworks.K8sIoClientGo

go/ql/lib/semmle/go/frameworks/JWT.qll

@@ -0,0 +1,91 @@
+/** Provides models of commonly used functions in common JWT packages. */
+
+import go
+
+/**
+ * Provides models of commonly used functions in common JWT packages.
+ */
+module JWT {
+  /** Gets the package name `github.com/lestrrat-go/jwx` v2. */
+  string packageLestrrat() { result = package("github.com/lestrrat-go/jwx/v2/jwt", "") }
+
+  /** Gets the package name `github.com/lestrrat-go/jwx` v1. */
+  string packageLestrratv1() { result = package("github.com/lestrrat-go/jwx/jwt", "") }
+
+  /** Gets the package name `github.com/golang-jwt/jwt` v4 and v5. */
+  string packagePathModern() {
+    result = package(["github.com/golang-jwt/jwt/v5", "github.com/golang-jwt/jwt/v4"], "")
+  }
+
+  /** Gets the package name `github.com/golang-jwt/jwt` v1 */
+  string packagePathOld() { result = package("github.com/golang-jwt/jwt", "") }
+
+  /**
+   * A function in golang-jwt to create new parser.
+   */
+  class NewParser extends Function {
+    NewParser() { this.hasQualifiedName(packagePathModern(), "NewParser") }
+  }
+
+  /**
+   * A function in golang-jwt to specify allowed algorithms.
+   */
+  class WithValidMethods extends Function {
+    WithValidMethods() { this.hasQualifiedName(packagePathModern(), "WithValidMethods") }
+  }
+
+  /**
+   * The methods to parse token that check token signature.
+   */
+  class SafeJwtParserMethod extends Method {
+    SafeJwtParserMethod() {
+      this.hasQualifiedName(packagePathModern(), "Parser", ["Parse", "ParseWithClaims"])
+    }
+  }
+
+  /**
+   * The function to parse token that check token signature.
+   */
+  class SafeJwtParserFunc extends Function {
+    SafeJwtParserFunc() {
+      this.hasQualifiedName([packagePathModern(), packagePathOld()], ["Parse", "ParseWithClaims"])
+    }
+  }
+
+  /**
+   * The methods to parse token that don't token signature.
+   */
+  class UnafeJwtParserMethod extends Method {
+    UnafeJwtParserMethod() {
+      this.hasQualifiedName(packagePathModern(), "Parser", "ParseUnverified")
+    }
+  }
+
+  /**
+   * A v2 Function to parse token that check token signature.
+   */
+  class LestrratParse extends Function {
+    LestrratParse() { this.hasQualifiedName(packageLestrrat(), "Parse") }
+  }
+
+  /**
+   * A v1 Function to parse token that check token signature.
+   */
+  class LestrratParsev1 extends Function {
+    LestrratParsev1() { this.hasQualifiedName(packageLestrratv1(), "Parse") }
+  }
+
+  /**
+   * A function included as parse option to verify token.
+   */
+  class LestrratVerify extends Function {
+    LestrratVerify() { this.hasQualifiedName(packageLestrratv1(), "WithVerify") }
+  }
+
+  /**
+   * The function to parse token that doesn't check signature.
+   */
+  class LestrratParseInsecure extends Function {
+    LestrratParseInsecure() { this.hasQualifiedName(packageLestrrat(), "ParseInsecure") }
+  }
+}

go/ql/src/change-notes/2023-09-13-JWT-signature-queries.md

@@ -0,0 +1,5 @@
+---
+category: newQuery
+---
+* Added JWT Confusion query `go/jwt-alg-confusion`
+* Added JWT Parsing without Signature Check query `go/jwt-insecure-signing`

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,41 @@
+/**
+ * @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 JWT
+import DataFlow
+
+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(packagePathModern(), "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"

go/ql/src/experimental/CWE-347/JWTParsingSignature.ql

@@ -0,0 +1,29 @@
+/**
+ * @name JWT Insecure Parsing
+ * @description Parsing JWT tokens without checking the signature may result in an authentication bypass
+ * @kind problem
+ * @problem.severity error
+ * @security-severity 8.0
+ * @precision medium
+ * @id go/jwt-insecure-signing
+ * @tags security
+ *       external/cwe/cwe-347
+ */
+
+import go
+import JWT
+import DataFlow
+
+from CallNode c
+where
+  c.getTarget() instanceof LestrratParseInsecure
+  or
+  c.getTarget() instanceof UnafeJwtParserMethod
+  or
+  c.getTarget() instanceof LestrratParsev1 and
+  not exists(LestrratVerify lv |
+    c.getCall().getAnArgument() = lv.getACall().asExpr() and
+    not c.getCall().getArgument(0) = lv.getACall().asExpr()
+  )
+select c,
+  "This call to Parse to accept JWT token does not check the signature, which may allow an attacker to forget tokens"

go/ql/test/experimental/CWE-347/JWTInsecureParsingLestrratv2.go

@@ -0,0 +1,37 @@
+package main
+
+import (
+	"time"
+
+	"github.com/lestrrat-go/jwx/v2/jwa"
+	"github.com/lestrrat-go/jwx/v2/jwk"
+	"github.com/lestrrat-go/jwx/v2/jwt"
+)
+
+func ExampleJWT_Parse2() {
+	jwkSymmetricKey, _ := jwk.FromRaw([]byte(`abracadabra`))
+	tok, err := jwt.NewBuilder().
+		Issuer(`github.com/lestrrat-go/jwx`).
+		IssuedAt(time.Now().Add(-5 * time.Minute)).
+		Expiration(time.Now().Add(time.Hour)).
+		Build()
+	v, err := jwt.Sign(tok, jwt.WithKey(jwa.HS256, jwkSymmetricKey))
+	if err != nil {
+		return
+	}
+	jwtSignedWithHS256 := v
+	tok, err = jwt.Parse(jwtSignedWithHS256)
+	if err != nil {
+		return
+	}
+	tok, err = jwt.Parse(jwtSignedWithHS256, jwt.WithKey(jwa.HS256, jwkSymmetricKey))
+	if err != nil {
+		return
+	}
+	tok, err = jwt.ParseInsecure(jwtSignedWithHS256)
+	if err != nil {
+		return
+	}
+	_ = tok
+	// OUTPUT:
+}

go/ql/test/experimental/CWE-347/JWTInsecureParsingLesttrat.go

@@ -0,0 +1,39 @@
+package main
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/lestrrat-go/jwx/jwa"
+	"github.com/lestrrat-go/jwx/jwt"
+)
+
+func ExampleJWT_Parse() {
+	jwkSymmetricKey := []byte(`abracadabra`)
+	tok, err := jwt.NewBuilder().
+		Issuer(`github.com/lestrrat-go/jwx`).
+		IssuedAt(time.Now().Add(-5 * time.Minute)).
+		Expiration(time.Now().Add(time.Hour)).
+		Build()
+	alg := jwa.HS256
+	v, err := jwt.Sign(tok, alg, jwkSymmetricKey)
+	if err != nil {
+		fmt.Errorf(`failed to sign token with HS256: %w`, err)
+		return
+	}
+	jwtSignedWithHS256 := v
+	//Bad
+	tok, err = jwt.Parse(jwtSignedWithHS256)
+	if err != nil {
+		fmt.Printf("%s\n", err)
+		return
+	}
+	//Good
+	tok, err = jwt.Parse(jwtSignedWithHS256, jwt.WithVerify(alg, jwkSymmetricKey))
+	if err != nil {
+		fmt.Printf("%s\n", err)
+		return
+	}
+	_ = tok
+	// OUTPUT:
+}

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

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