github · Kwstubbs · Aug 25, 2023 · Aug 29, 2023 · Aug 29, 2023 · Aug 29, 2023
@@ -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

@@ -0,0 +1,74 @@
+import go
+
+module JWT {
+
+string packageLestrrat() {
+    result =
+    package("github.com/lestrrat-go/jwx/v2/jwt", "")
+
+}
+string packageLestrratv1() {
+  result =
+    package("github.com/lestrrat-go/jwx/jwt", "")
+}
+string packagePathModern() {
+    result =
+      package(["github.com/golang-jwt/jwt/v5", "github.com/golang-jwt/jwt/v4"], "")
+  }
+  string packagePathOld(){
+    result =
+      package(
+          "github.com/golang-jwt/jwt", "")
+  }
+
+  class NewParser extends Function {
+    NewParser() {
+      this.hasQualifiedName([packagePathModern()], "NewParser")
+    }
+  }
+
+  class WithValidMethods extends Function{
+    WithValidMethods() {
+      this.hasQualifiedName([packagePathModern()], "WithValidMethods")
+    }
+  }
+
+  class SafeJWTParserMethod extends Method {
+    SafeJWTParserMethod() {
+      this.hasQualifiedName(packagePathModern(), "Parser", ["Parse", "ParseWithClaims"])
+    }
+  }
+
+  class SafeJWTParserFunc extends Function{
+    SafeJWTParserFunc(){
+    this.hasQualifiedName([packagePathModern(),packagePathOld()], ["Parse", "ParseWithClaims"])
+    }
+  }
+
+  class UnafeJWTParserMethod extends Method{
+    UnafeJWTParserMethod(){
+    this.hasQualifiedName(packagePathModern(), "Parser", "ParseUnverified")
+    }
+  }
+  class LestrratParse extends Function{
+    LestrratParse() {
+        this.hasQualifiedName(packageLestrrat(), "Parse")
+    }
+  }
+  class LestrratParsev1 extends Function{
+    LestrratParsev1() {
+        this.hasQualifiedName(packageLestrratv1(), "Parse")
+    }
+  }
+  class LestrratVerify extends Function {
+    LestrratVerify() {
+    this.hasQualifiedName(packageLestrratv1(), "WithVerify")
+    }
+  }
+  class LestrratParseInsecure extends Function{
+    LestrratParseInsecure() {
+        this.hasQualifiedName(packageLestrrat(), "ParseInsecure")
+    }
+  }
+
+}
@@ -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
+}
@@ -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,33 @@
+/**
+ * @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-pitfalls
+ * @tags security
+ *       external/cwe/cwe-347
+ */
+import go
+import JWT
+import DataFlow
+
+from CallNode c, NewParser m, WithValidMethods wvm, 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 | c2.getTarget() = m 
+and ( c2.getCall().getAnArgument() = wvm.getACall().asExpr()
+and DataFlow::localFlow(c2.getAResult(), c.getReceiver()))
+))
+//ParserFunc creates a new default Parser on call that accepts all methods
+or func instanceof SafeJWTParserFunc
+)
+//Check that the Parse(function or method) does not check the Token Method field, which most likely is a check for method type
+and 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"
@@ -0,0 +1,21 @@
+/**
+ * @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, LestrratParsev1 lp
+where c.getTarget() instanceof LestrratParseInsecure or c.getTarget() instanceof UnafeJWTParserMethod
+ or (c.getTarget() = lp 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"
+
+
@@ -0,0 +1,40 @@
+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_Parse() {
+	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:
+}
+func main() {
+	ExampleJWT_Parse()
+}
@@ -0,0 +1,42 @@
+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:
+}
+func main() {
+	ExampleJWT_Parse()
+}
@@ -0,0 +1,2 @@
+| JWTParsingAlgorithim.go:20:4:23:5 | call to Parse | This Parse Call to Verify the JWT token is vulnerable to algorithim confusion |
+| JWTParsingAlgorithim.go:38:22:41:5 | call to Parse | This Parse Call to Verify the JWT token is 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/JWTParsingAlgorithim.ql
@@ -0,0 +1,2 @@
+| JWTInsecureParsingLestrratv2.go:31:13:31:49 | call to ParseInsecure | This call to Parse to accept JWT token does not check the signature, which may allow an attacker to forget tokens |
+| JWTInsecureParsingLesttrat.go:26:13:26:41 | call to Parse | This call to Parse to accept JWT token does not check the signature, which may allow an attacker to forget tokens |
@@ -0,0 +1 @@
+experimental/CWE-347/JWTParsingSignature.ql
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		\| JWTParsingAlgorithim.go:20:4:23:5 \| call to Parse \| This Parse Call to Verify the JWT token is vulnerable to algorithim confusion \|
		\| JWTParsingAlgorithim.go:38:22:41:5 \| call to Parse \| This Parse Call to Verify the JWT token is vulnerable to algorithim confusion \|
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		\| JWTInsecureParsingLestrratv2.go:31:13:31:49 \| call to ParseInsecure \| This call to Parse to accept JWT token does not check the signature, which may allow an attacker to forget tokens \|
		\| JWTInsecureParsingLesttrat.go:26:13:26:41 \| call to Parse \| This call to Parse to accept JWT token does not check the signature, which may allow an attacker to forget tokens \|