Java: JWT decoding without verification [smowton fork]

java/ql/src/experimental/Security/CWE/CWE-347/Auth0NoVerifier.qhelp

@@ -0,0 +1,34 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+     <overview>
+          <p>
+               A JSON Web Token (JWT) is used for authenticating and managing users in an application.
+          </p>
+          <p>
+               Only Decoding JWTs without checking if they have a valid signature or not can lead to security vulnerabilities.
+          </p>
+
+     </overview>
+     <recommendation>
+
+          <p>
+               Don't use methods that only decode JWT, Instead use methods that verify the signature of JWT.
+          </p>
+
+     </recommendation>
+     <example>
+
+          <p>
+               The following code you can see an Example from a popular Library.
+          </p>
+
+          <sample src="Example.java" />
+
+     </example>
+     <references>
+          <li>
+               <a href="CVE-2021-37580">The incorrect use of JWT in ShenyuAdminBootstrap allows an attacker to bypass authentication.</a>
+          </li>
+     </references>
+
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-347/Auth0NoVerifier.ql

@@ -0,0 +1,136 @@
+/**
+ * @name Missing JWT signature check
+ * @description Failing to check the Json Web Token (JWT) signature may allow an attacker to forge their own tokens.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.8
+ * @precision high
+ * @id java/missing-jwt-signature-check
+ * @tags security
+ *       external/cwe/cwe-347
+ */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.FlowSources
+
+module JwtAuth0 {
+  class PayloadType extends RefType {
+    PayloadType() { this.hasQualifiedName("com.auth0.jwt.interfaces", "Payload") }
+  }
+
+  class JWTType extends RefType {
+    JWTType() { this.hasQualifiedName("com.auth0.jwt", "JWT") }
+  }
+
+  class JWTVerifierType extends RefType {
+    JWTVerifierType() { this.hasQualifiedName("com.auth0.jwt", "JWTVerifier") }
+  }
+
+  /**
+   * A Method that returns a Decoded Claim of JWT
+   */
+  class GetPayload extends MethodAccess {
+    GetPayload() {
+      this.getCallee().getDeclaringType() instanceof PayloadType and
+      this.getCallee().hasName(["getClaim", "getIssuedAt"])
+    }
+  }
+
+  /**
+   * A Method that Decode JWT without signature verification
+   */
+  class Decode extends MethodAccess {
+    Decode() {
+      this.getCallee().getDeclaringType() instanceof JWTType and
+      this.getCallee().hasName("decode")
+    }
+  }
+
+  /**
+   * A Method that Decode JWT with signature verification
+   */
+  class Verify extends MethodAccess {
+    Verify() {
+      this.getCallee().getDeclaringType() instanceof JWTVerifierType and
+      this.getCallee().hasName("verify")
+    }
+  }
+}
+
+module JwtDecodeConfig implements DataFlow::StateConfigSig {
+  class FlowState = DataFlow::FlowState;
+
+  predicate isSource(DataFlow::Node source, FlowState state) {
+    (
+      exists(Variable v |
+        source.asExpr() = v.getInitializer() and
+        v.getType().hasName("String")
+      )
+      or
+      source instanceof RemoteFlowSource
+    ) and
+    not FlowToJwtVerify::flow(source, _) and
+    state = "Auth0" and
+    not state = "Auth0Verify"
+  }
+
+  predicate isSink(DataFlow::Node sink, FlowState state) {
+    sink.asExpr() = any(JwtAuth0::GetPayload a) and
+    state = "Auth0" and
+    not state = "Auth0Verify"
+  }
+
+  predicate isAdditionalFlowStep(
+    DataFlow::Node nodeFrom, FlowState stateFrom, DataFlow::Node nodeTo, FlowState stateTo
+  ) {
+    // Decode Should be one of the middle nodes
+    exists(JwtAuth0::Decode a |
+      nodeFrom.asExpr() = a.getArgument(0) and
+      nodeTo.asExpr() = a and
+      stateTo = "Auth0" and
+      stateFrom = "Auth0"
+    )
+    or
+    exists(JwtAuth0::Verify a |
+      nodeFrom.asExpr() = a.getArgument(0) and
+      nodeTo.asExpr() = a and
+      stateTo = "Auth0Verify" and
+      stateFrom = "Auth0Verify"
+    )
+    or
+    exists(JwtAuth0::GetPayload a |
+      nodeFrom.asExpr() = a.getQualifier() and
+      nodeTo.asExpr() = a and
+      stateTo = "Auth0" and
+      stateFrom = "Auth0"
+    )
+  }
+
+  predicate isBarrier(DataFlow::Node sanitizer, FlowState state) { none() }
+}
+
+module FlowToJwtVerifyConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    // source instanceof DataFlow::Node
+    exists(Variable v |
+      source.asExpr() = v.getInitializer() and
+      v.getType().hasName("String")
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(JwtAuth0::Verify a).getArgument(0) }
+
+  predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { none() }
+}
+
+module JwtDecode = TaintTracking::GlobalWithState<JwtDecodeConfig>;
+
+module FlowToJwtVerify = TaintTracking::Global<FlowToJwtVerifyConfig>;
+
+import JwtDecode::PathGraph
+
+from JwtDecode::PathNode source, JwtDecode::PathNode sink
+where JwtDecode::flowPath(source, sink)
+select sink.getNode(), source, sink, "This parses a $@, but the signature is not verified.",
+  source.getNode(), "JWT"

java/ql/src/experimental/Security/CWE/CWE-347/Example.java

@@ -0,0 +1,80 @@
+package com.example.JwtTest;
+
+import java.io.*;
+import java.security.NoSuchAlgorithmException;
+import java.util.Objects;
+import java.util.Optional;
+import javax.crypto.KeyGenerator;
+import javax.servlet.http.*;
+import javax.servlet.annotation.*;
+import com.auth0.jwt.JWT;
+import com.auth0.jwt.JWTVerifier;
+import com.auth0.jwt.algorithms.Algorithm;
+import com.auth0.jwt.exceptions.JWTCreationException;
+import com.auth0.jwt.exceptions.JWTVerificationException;
+import com.auth0.jwt.interfaces.DecodedJWT;
+
+@WebServlet(name = "JwtTest1", value = "/Auth")
+public class auth0 extends HttpServlet {
+
+  public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException {
+    response.setContentType("text/html");
+    PrintWriter out = response.getWriter();
+
+    // OK: first decode without signature verification
+    // and then verify with signature verification
+    String JwtToken1 = request.getParameter("JWT1");
+    String userName =  decodeToken(JwtToken1);
+    verifyToken(JwtToken1, "A Securely generated Key");
+    if (Objects.equals(userName, "Admin")) {
+      out.println("<html><body>");
+      out.println("<h1>" + "heyyy Admin" + "</h1>");
+      out.println("</body></html>");
+    }
+
+    out.println("<html><body>");
+    out.println("<h1>" + "heyyy Nobody" + "</h1>");
+    out.println("</body></html>");
+  }
+
+  public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException {
+    response.setContentType("text/html");
+    PrintWriter out = response.getWriter();
+
+    // NOT OK:  only decode, no verification
+    String JwtToken2 = request.getParameter("JWT2");
+    String userName = decodeToken(JwtToken2);
+    if (Objects.equals(userName, "Admin")) {
+      out.println("<html><body>");
+      out.println("<h1>" + "heyyy Admin" + "</h1>");
+      out.println("</body></html>");
+    }
+
+    // OK:  no clue of the use of unsafe decoded JWT return value
+    JwtToken2 = request.getParameter("JWT2");
+    JWT.decode(JwtToken2);
+
+
+    out.println("<html><body>");
+    out.println("<h1>" + "heyyy Nobody" + "</h1>");
+    out.println("</body></html>");
+  }
+
+  public static boolean verifyToken(final String token, final String key) {
+    try {
+      JWTVerifier verifier = JWT.require(Algorithm.HMAC256(key)).build();
+      verifier.verify(token);
+      return true;
+    } catch (JWTVerificationException e) {
+      System.out.printf("jwt decode fail, token: %s", e);
+    }
+    return false;
+  }
+
+
+  public static String decodeToken(final String token) {
+    DecodedJWT jwt = JWT.decode(token);
+    return Optional.of(jwt).map(item -> item.getClaim("userName").asString()).orElse("");
+  }
+
+}

java/ql/test/experimental/query-tests/security/CWE-347/Auth0NoVerifier.expected

@@ -0,0 +1,28 @@
+WARNING: type 'FlowState' has been deprecated and may be removed in future (Auth0NoVerifier.ql:62,21-40)
+WARNING: type 'MethodAccess' has been deprecated and may be removed in future (Auth0NoVerifier.ql:33,28-40)
+WARNING: type 'MethodAccess' has been deprecated and may be removed in future (Auth0NoVerifier.ql:43,24-36)
+WARNING: type 'MethodAccess' has been deprecated and may be removed in future (Auth0NoVerifier.ql:53,24-36)
+edges
+| JwtNoVerifier.java:42:28:42:55 | getParameter(...) : String | JwtNoVerifier.java:43:39:43:47 | JwtToken2 : String | provenance | Src:MaD:44684  |
+| JwtNoVerifier.java:43:39:43:47 | JwtToken2 : String | JwtNoVerifier.java:72:38:72:55 | token : String | provenance |  |
+| JwtNoVerifier.java:72:38:72:55 | token : String | JwtNoVerifier.java:73:37:73:41 | token : String | provenance |  |
+| JwtNoVerifier.java:73:26:73:42 | decode(...) : DecodedJWT | JwtNoVerifier.java:74:28:74:30 | jwt : DecodedJWT | provenance |  |
+| JwtNoVerifier.java:73:37:73:41 | token : String | JwtNoVerifier.java:73:26:73:42 | decode(...) : DecodedJWT | provenance | Config |
+| JwtNoVerifier.java:74:16:74:31 | of(...) : Optional [<element>] : DecodedJWT | JwtNoVerifier.java:74:37:74:40 | item : DecodedJWT | provenance | MaD:43977 |
+| JwtNoVerifier.java:74:28:74:30 | jwt : DecodedJWT | JwtNoVerifier.java:74:16:74:31 | of(...) : Optional [<element>] : DecodedJWT | provenance | MaD:43979 |
+| JwtNoVerifier.java:74:37:74:40 | item : DecodedJWT | JwtNoVerifier.java:74:45:74:48 | item : DecodedJWT | provenance |  |
+| JwtNoVerifier.java:74:45:74:48 | item : DecodedJWT | JwtNoVerifier.java:74:45:74:69 | getClaim(...) | provenance | Config |
+nodes
+| JwtNoVerifier.java:42:28:42:55 | getParameter(...) : String | semmle.label | getParameter(...) : String |
+| JwtNoVerifier.java:43:39:43:47 | JwtToken2 : String | semmle.label | JwtToken2 : String |
+| JwtNoVerifier.java:72:38:72:55 | token : String | semmle.label | token : String |
+| JwtNoVerifier.java:73:26:73:42 | decode(...) : DecodedJWT | semmle.label | decode(...) : DecodedJWT |
+| JwtNoVerifier.java:73:37:73:41 | token : String | semmle.label | token : String |
+| JwtNoVerifier.java:74:16:74:31 | of(...) : Optional [<element>] : DecodedJWT | semmle.label | of(...) : Optional [<element>] : DecodedJWT |
+| JwtNoVerifier.java:74:28:74:30 | jwt : DecodedJWT | semmle.label | jwt : DecodedJWT |
+| JwtNoVerifier.java:74:37:74:40 | item : DecodedJWT | semmle.label | item : DecodedJWT |
+| JwtNoVerifier.java:74:45:74:48 | item : DecodedJWT | semmle.label | item : DecodedJWT |
+| JwtNoVerifier.java:74:45:74:69 | getClaim(...) | semmle.label | getClaim(...) |
+subpaths
+#select
+| JwtNoVerifier.java:74:45:74:69 | getClaim(...) | JwtNoVerifier.java:42:28:42:55 | getParameter(...) : String | JwtNoVerifier.java:74:45:74:69 | getClaim(...) | This parses a $@, but the signature is not verified. | JwtNoVerifier.java:42:28:42:55 | getParameter(...) | JWT |

java/ql/test/experimental/query-tests/security/CWE-347/Auth0NoVerifier.qlref

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