Merge pull request #15585 from atorralba/atorralba/go/promote-jwt-unsafe-verification

Go: Promote `go/missing-jwt-signature-check` from experimental

Author: atorralba

go/ql/lib/ext/github.com.dgrijalva.jwt-go.model.yml

@@ -3,5 +3,19 @@ extensions:
       pack: codeql/go-all
       extensible: sinkModel
     data:
-      - ["github.com/dgrijalva/jwt-go", "Token", True, "SignedString", "", "", "Argument[0]", "credentials-key", "manual"]
       - ["github.com/dgrijalva/jwt-go", "SigningMethod", True, "Sign", "", "", "Argument[1]", "credentials-key", "manual"]
+      - ["github.com/dgrijalva/jwt-go", "Token", True, "SignedString", "", "", "Argument[0]", "credentials-key", "manual"]
+      - ["github.com/dgrijalva/jwt-go", "Parser", True, "ParseUnverified", "", "", "Argument[0]", "jwt", "manual"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: summaryModel
+    data:
+      - ["github.com/dgrijalva/jwt-go", "", True, "Parse", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/dgrijalva/jwt-go", "Parser", True, "Parse", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/dgrijalva/jwt-go", "", True, "ParseWithClaims", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/dgrijalva/jwt-go", "Parser", True, "ParseWithClaims", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/dgrijalva/jwt-go", "", True, "ParseECPrivateKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/dgrijalva/jwt-go", "", True, "ParseECPublicKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/dgrijalva/jwt-go", "", True, "ParseRSAPrivateKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/dgrijalva/jwt-go", "", True, "ParseRSAPrivateKeyFromPEMWithPassword", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/dgrijalva/jwt-go", "", True, "ParseRSAPublicKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]

go/ql/lib/ext/github.com.go-jose.go-jose.model.yml

@@ -0,0 +1,14 @@
+extensions:
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sinkModel
+    data:
+      - ["github.com/go-jose/go-jose/$ANYVERSION/jwt", "JSONWebToken", True, "UnsafeClaimsWithoutVerification", "", "", "Argument[-1]", "jwt", "manual"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: summaryModel
+    data:
+      - ["github.com/go-jose/go-jose/$ANYVERSION/jwt", "", True, "ParseEncrypted", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/go-jose/go-jose/$ANYVERSION/jwt", "", True, "ParseSigned", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/go-jose/go-jose/$ANYVERSION/jwt", "NestedJSONWebToken", True, "ParseSignedAndEncrypted", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/go-jose/go-jose/$ANYVERSION/jwt", "NestedJSONWebToken", True, "Decrypt", "", "", "Argument[-1]", "ReturnValue[0]", "taint", "manual"]

go/ql/lib/ext/github.com.golang-jwt.jwt.model.yml

@@ -3,5 +3,21 @@ extensions:
       pack: codeql/go-all
       extensible: sinkModel
     data:
-      - ["github.com/golang-jwt/jwt", "Token", True, "SignedString", "", "", "Argument[0]", "credentials-key", "manual"]
       - ["github.com/golang-jwt/jwt", "SigningMethod", True, "Sign", "", "", "Argument[1]", "credentials-key", "manual"]
+      - ["github.com/golang-jwt/jwt", "Token", True, "SignedString", "", "", "Argument[0]", "credentials-key", "manual"]
+      - ["github.com/golang-jwt/jwt", "Parser", True, "ParseUnverified", "", "", "Argument[0]", "jwt", "manual"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: summaryModel
+    data:
+      - ["github.com/golang-jwt/jwt", "", True, "Parse", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/golang-jwt/jwt", "Parser", True, "Parse", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/golang-jwt/jwt", "", True, "ParseWithClaims", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/golang-jwt/jwt", "Parser", True, "ParseWithClaims", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/golang-jwt/jwt", "", True, "ParseECPrivateKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/golang-jwt/jwt", "", True, "ParseECPublicKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/golang-jwt/jwt", "", True, "ParseEdPrivateKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/golang-jwt/jwt", "", True, "ParseEdPublicKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/golang-jwt/jwt", "", True, "ParseRSAPrivateKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/golang-jwt/jwt", "", True, "ParseRSAPublicKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["github.com/golang-jwt/jwt", "", True, "RegisterSigningMethod", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]

go/ql/lib/ext/gopkg.in.square.go-jose.model.yml

@@ -0,0 +1,14 @@
+extensions:
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sinkModel
+    data:
+      - ["gopkg.in/square/go-jose.v2/jwt", "JSONWebToken", True, "UnsafeClaimsWithoutVerification", "", "", "Argument[-1]", "jwt", "manual"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: summaryModel
+    data:
+      - ["gopkg.in/square/go-jose.v2/jwt", "", True, "ParseEncrypted", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["gopkg.in/square/go-jose.v2/jwt", "", True, "ParseSigned", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["gopkg.in/square/go-jose.v2/jwt", "NestedJSONWebToken", True, "ParseSignedAndEncrypted", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+      - ["gopkg.in/square/go-jose.v2/jwt", "NestedJSONWebToken", True, "Decrypt", "", "", "Argument[-1]", "ReturnValue[0]", "taint", "manual"]

go/ql/lib/go.qll

@@ -52,6 +52,7 @@ import semmle.go.frameworks.GoMicro
 import semmle.go.frameworks.GoRestfulHttp
 import semmle.go.frameworks.Gqlgen
 import semmle.go.frameworks.Iris
+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/GoJose.qll

@@ -9,16 +9,40 @@ private import semmle.go.security.HardcodedCredentials
 private module GoJose {
   private class GoJoseKey extends HardcodedCredentials::Sink {
     GoJoseKey() {
-      exists(Field f, string pkg |
-        pkg =
-          [
-            package("github.com/square/go-jose", ""), package("github.com/go-jose/go-jose", ""),
-            "gopkg.in/square/go-jose.v2"
-          ]
-      |
-        f.hasQualifiedName(pkg, ["Recipient", "SigningKey"], "Key") and
+      exists(Field f |
+        f.hasQualifiedName(goJosePackage(), ["Recipient", "SigningKey"], "Key") and
         f.getAWrite().getRhs() = this
       )
     }
   }
+
+  private string goJosePackage() {
+    result =
+      [
+        package("github.com/square/go-jose", ""), package("github.com/go-jose/go-jose", ""),
+        "gopkg.in/square/go-jose.v2"
+      ]
+  }
+
+  /**
+   * Provides classes and predicates for working with the `gopkg.in/square/go-jose/jwt` and
+   * `github.com/go-jose/go-jose/jwt` packages.
+   */
+  private module Jwt {
+    private import semmle.go.security.MissingJwtSignatureCheckCustomizations::MissingJwtSignatureCheck
+
+    /** The method `JSONWebToken.Claims`. */
+    private class GoJoseParseWithClaims extends JwtSafeParse {
+      GoJoseParseWithClaims() {
+        this.(Method).hasQualifiedName(goJoseJwtPackage(), "JSONWebToken", "Claims")
+      }
+
+      override int getTokenArgNum() { result = -1 }
+    }
+
+    /** Gets the package names `gopkg.in/square/go-jose/jwt` and `github.com/go-jose/go-jose/jwt`. */
+    private string goJoseJwtPackage() {
+      result = package(["gopkg.in/square/go-jose", "github.com/go-jose/go-jose"], "jwt")
+    }
+  }
 }

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

@@ -0,0 +1,57 @@
+/**
+ * Provides classes and predicates for working with the `github.com/golang-jwt/jwt` and
+ * `github.com/dgrijalva/jwt-go` packages.
+ */
+
+import go
+private import semmle.go.security.MissingJwtSignatureCheckCustomizations::MissingJwtSignatureCheck
+
+/** The function `jwt.Parse` or the method `Parser.Parse`. */
+private class GolangJwtParse extends JwtSafeParse {
+  GolangJwtParse() {
+    this.hasQualifiedName(golangJwtPackage(), "Parse")
+    or
+    this.(Method).hasQualifiedName(golangJwtPackage(), "Parser", "Parse")
+  }
+
+  override int getTokenArgNum() { result = 0 }
+}
+
+/** The function `jwt.ParseWithClaims` or the method `Parser.ParseWithClaims`. */
+private class GolangJwtParseWithClaims extends JwtSafeParse {
+  GolangJwtParseWithClaims() {
+    this.hasQualifiedName(golangJwtPackage(), "ParseWithClaims")
+    or
+    this.(Method).hasQualifiedName(golangJwtPackage(), "Parser", "ParseWithClaims")
+  }
+
+  override int getTokenArgNum() { result = 0 }
+}
+
+/** The function `jwt.ParseFromRequest`. */
+private class GolangJwtParseFromRequest extends JwtSafeParse {
+  GolangJwtParseFromRequest() {
+    this.hasQualifiedName(golangJwtRequestPackage(), "ParseFromRequest")
+  }
+
+  override int getTokenArgNum() { result = 0 }
+}
+
+/** The function `jwt.ParseFromRequestWithClaims`. */
+private class GolangJwtParseFromRequestWithClaims extends JwtSafeParse {
+  GolangJwtParseFromRequestWithClaims() {
+    this.hasQualifiedName(golangJwtRequestPackage(), "ParseFromRequestWithClaims")
+  }
+
+  override int getTokenArgNum() { result = 0 }
+}
+
+/** Gets the pakcage names `github.com/golang-jwt/jwt` and `github.com/dgrijalva/jwt-go`. */
+private string golangJwtPackage() {
+  result = package(["github.com/golang-jwt/jwt", "github.com/dgrijalva/jwt-go"], "")
+}
+
+/** Gets the package names `github.com/golang-jwt/jwt/request` and `github.com/dgrijalva/jwt-go/request`. */
+private string golangJwtRequestPackage() {
+  result = package(["github.com/golang-jwt/jwt", "github.com/dgrijalva/jwt-go"], "request")
+}

go/ql/lib/semmle/go/security/MissingJwtSignatureCheck.qll

@@ -0,0 +1,42 @@
+/**
+ * Provides a taint tracking flow for reasoning about JWT vulnerabilities.
+ *
+ * Note: for performance reasons, only import this file if `MissingJwtSignatureCheck::Config` or `MissingJwtSignatureCheck::Flow` are needed,
+ * otherwise `MissingJwtSignatureCheckCustomizations` should be imported instead.
+ */
+
+import go
+
+/** Provides a taint-tracking flow for reasoning about JWT vulnerabilities. */
+module MissingJwtSignatureCheck {
+  import MissingJwtSignatureCheckCustomizations::MissingJwtSignatureCheck
+
+  /** Config for reasoning about JWT vulnerabilities. */
+  module Config implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) {
+      source instanceof Source and
+      not SafeParse::flow(source, _)
+    }
+
+    predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+    predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+      any(AdditionalFlowStep s).step(nodeFrom, nodeTo)
+    }
+  }
+
+  /** Tracks taint flow for reasoning about JWT vulnerabilities. */
+  module Flow = TaintTracking::Global<Config>;
+
+  private module SafeParseConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    predicate isSink(DataFlow::Node sink) { sink = any(JwtSafeParse jwtParse).getTokenArg() }
+
+    predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+      any(AdditionalFlowStep s).step(nodeFrom, nodeTo)
+    }
+  }
+
+  private module SafeParse = TaintTracking::Global<SafeParseConfig>;
+}

go/ql/lib/semmle/go/security/MissingJwtSignatureCheckCustomizations.qll

@@ -0,0 +1,57 @@
+/**
+ * Provides default sources, sinks, and sanitizers for reasoning about
+ * JWT vulnerabilities, as well as extension points for adding your own.
+ */
+
+import go
+private import semmle.go.dataflow.ExternalFlow
+private import codeql.util.Unit
+
+/**
+ * Provides extension points for customizing the data-flow tracking configuration for reasoning
+ * about JWT vulnerabilities.
+ */
+module MissingJwtSignatureCheck {
+  /**
+   * A data flow source for JWT vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for JWT vulnerabilities.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for JWT vulnerabilities.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /** An additional flow step for JWT vulnerabilities. */
+  class AdditionalFlowStep extends Unit {
+    /**
+     * Holds if the step from `node1` to `node2` should be considered a flow
+     * step for configurations related to JWT vulnerabilities.
+     */
+    abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
+  }
+
+  /** A function that parses and correctly validates a JWT token. */
+  abstract class JwtSafeParse extends Function {
+    /** Gets the position of the JWT argument in a call to this function. */
+    abstract int getTokenArgNum();
+
+    /** Gets the JWT argument of a call to this function. */
+    DataFlow::Node getTokenArg() {
+      this.getTokenArgNum() != -1 and result = this.getACall().getArgument(this.getTokenArgNum())
+      or
+      this.getTokenArgNum() = -1 and result = this.getACall().getReceiver()
+    }
+  }
+
+  private class DefaultSource extends Source instanceof UntrustedFlowSource { }
+
+  private class DefaultSink extends Sink {
+    DefaultSink() { sinkNode(this, "jwt") }
+  }
+}

go/ql/src/Security/CWE-347/MissingJwtSignatureCheck.qhelp

@@ -0,0 +1,25 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+     <overview>
+          <p>Applications decoding a JSON Web Token (JWT) may be vulnerable when the
+               signature is not correctly verified.</p>
+     </overview>
+     <recommendation>
+          <p>Always verify the signature by using the appropriate methods provided by the JWT
+               library, or use a library that verifies it by default.</p>
+     </recommendation>
+     <example>
+          <p>The following (bad) example shows a case where a JWT is parsed without verifying the
+               signature.</p>
+          <sample src="MissingJwtSignatureCheckBad.go" />
+          <p>The following (good) example uses the appropriate function for parsing a JWT
+               and verifying its signature.</p>
+          <sample src="MissingJwtSignatureCheckGood.go" />
+     </example>
+     <references>
+          <li>JWT IO: <a href="https://jwt.io/introduction">Introduction to JSON Web Tokens</a>.</li>
+          <li>jwt-go: <a href="https://pkg.go.dev/github.com/golang-jwt/jwt/v5">Documentation</a>.</li>
+          <li>Go JOSE: <a href="https://pkg.go.dev/github.com/go-jose/go-jose/v3">Documentation</a>.</li>
+     </references>
+
+</qhelp>