Merge pull request #14089 from am0o0/amammad-java-JWT

Java: JWT decoding without verification

java/ql/lib/ext/experimental/org.apache.shiro.authc.model.yml

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sourceModel
+    data:
+      - ["org.apache.shiro.authc","AuthenticationToken",true,"getCredentials","()","","ReturnValue","remote","manual"]

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

@@ -0,0 +1,31 @@
+<!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. It must be verified in order to ensure the JWT is genuine.
+          </p>
+
+     </overview>
+     <recommendation>
+
+          <p>
+               Don't use information from a JWT without verifying that JWT.
+          </p>
+
+     </recommendation>
+     <example>
+
+          <p>
+               The following example illustrates secure and insecure use of the Auth0 `java-jwt` library.
+          </p>
+
+          <sample src="Example.java" />
+
+     </example>
+     <references>
+          <li>
+               <a href="https://nvd.nist.gov/vuln/detail/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,59 @@
+/**
+ * @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-auth0
+ * @tags security
+ *       external/cwe/cwe-347
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import JwtAuth0 as JwtAuth0
+
+module JwtDecodeConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source instanceof RemoteFlowSource and
+    not FlowToJwtVerify::flow(source, _)
+  }
+
+  predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(JwtAuth0::GetPayload a) }
+
+  predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    // Decode Should be one of the middle nodes
+    exists(JwtAuth0::Decode a |
+      nodeFrom.asExpr() = a.getArgument(0) and
+      nodeTo.asExpr() = a
+    )
+    or
+    exists(JwtAuth0::Verify a |
+      nodeFrom.asExpr() = a.getArgument(0) and
+      nodeTo.asExpr() = a
+    )
+    or
+    exists(JwtAuth0::GetPayload a |
+      nodeFrom.asExpr() = a.getQualifier() and
+      nodeTo.asExpr() = a
+    )
+  }
+}
+
+module FlowToJwtVerifyConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(JwtAuth0::Verify a).getArgument(0) }
+}
+
+module JwtDecode = TaintTracking::Global<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/src/experimental/Security/CWE/CWE-347/JwtAuth0.qll

@@ -0,0 +1,43 @@
+import java
+
+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 MethodCall {
+  GetPayload() {
+    this.getCallee().getDeclaringType() instanceof PayloadType and
+    this.getCallee().hasName(["getClaim", "getIssuedAt"])
+  }
+}
+
+/**
+ * A Method that Decode JWT without signature verification
+ */
+class Decode extends MethodCall {
+  Decode() {
+    this.getCallee().getDeclaringType() instanceof JwtType and
+    this.getCallee().hasName("decode")
+  }
+}
+
+/**
+ * A Method that Decode JWT with signature verification
+ */
+class Verify extends MethodCall {
+  Verify() {
+    this.getCallee().getDeclaringType() instanceof JwtVerifierType and
+    this.getCallee().hasName("verify")
+  }
+}

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

@@ -0,0 +1,36 @@
+#select
+| JwtNoVerifier.java:91:45:91:69 | getClaim(...) | JwtNoVerifier.java:44:28:44:55 | getParameter(...) : String | JwtNoVerifier.java:91:45:91:69 | getClaim(...) | This parses a $@, but the signature is not verified. | JwtNoVerifier.java:44:28:44:55 | getParameter(...) | JWT |
+| JwtNoVerifier.java:91:45:91:69 | getClaim(...) | JwtNoVerifier.java:58:37:58:62 | getCredentials(...) : Object | JwtNoVerifier.java:91:45:91:69 | getClaim(...) | This parses a $@, but the signature is not verified. | JwtNoVerifier.java:58:37:58:62 | getCredentials(...) | JWT |
+edges
+| JwtNoVerifier.java:44:28:44:55 | getParameter(...) : String | JwtNoVerifier.java:45:39:45:47 | JwtToken1 : String | provenance | Src:MaD:4  |
+| JwtNoVerifier.java:45:39:45:47 | JwtToken1 : String | JwtNoVerifier.java:89:38:89:55 | token : String | provenance |  |
+| JwtNoVerifier.java:58:28:58:62 | (...)... : String | JwtNoVerifier.java:59:32:59:40 | JwtToken3 : String | provenance |  |
+| JwtNoVerifier.java:58:37:58:62 | getCredentials(...) : Object | JwtNoVerifier.java:58:28:58:62 | (...)... : String | provenance | Src:MaD:1  |
+| JwtNoVerifier.java:59:32:59:40 | JwtToken3 : String | JwtNoVerifier.java:89:38:89:55 | token : String | provenance |  |
+| JwtNoVerifier.java:89:38:89:55 | token : String | JwtNoVerifier.java:90:37:90:41 | token : String | provenance |  |
+| JwtNoVerifier.java:90:26:90:42 | decode(...) : DecodedJWT | JwtNoVerifier.java:91:28:91:30 | jwt : DecodedJWT | provenance |  |
+| JwtNoVerifier.java:90:37:90:41 | token : String | JwtNoVerifier.java:90:26:90:42 | decode(...) : DecodedJWT | provenance | Config |
+| JwtNoVerifier.java:91:16:91:31 | of(...) : Optional [<element>] : DecodedJWT | JwtNoVerifier.java:91:37:91:40 | item : DecodedJWT | provenance | MaD:2 |
+| JwtNoVerifier.java:91:28:91:30 | jwt : DecodedJWT | JwtNoVerifier.java:91:16:91:31 | of(...) : Optional [<element>] : DecodedJWT | provenance | MaD:3 |
+| JwtNoVerifier.java:91:37:91:40 | item : DecodedJWT | JwtNoVerifier.java:91:45:91:48 | item : DecodedJWT | provenance |  |
+| JwtNoVerifier.java:91:45:91:48 | item : DecodedJWT | JwtNoVerifier.java:91:45:91:69 | getClaim(...) | provenance | Config |
+models
+| 1 | Source: org.apache.shiro.authc; AuthenticationToken; true; getCredentials; (); ; ReturnValue; remote; manual |
+| 2 | Summary: java.util; Optional; false; map; ; ; Argument[this].Element; Argument[0].Parameter[0]; value; manual |
+| 3 | Summary: java.util; Optional; false; of; ; ; Argument[0]; ReturnValue.Element; value; manual |
+| 4 | Source: javax.servlet; ServletRequest; false; getParameter; (String); ; ReturnValue; remote; manual |
+nodes
+| JwtNoVerifier.java:44:28:44:55 | getParameter(...) : String | semmle.label | getParameter(...) : String |
+| JwtNoVerifier.java:45:39:45:47 | JwtToken1 : String | semmle.label | JwtToken1 : String |
+| JwtNoVerifier.java:58:28:58:62 | (...)... : String | semmle.label | (...)... : String |
+| JwtNoVerifier.java:58:37:58:62 | getCredentials(...) : Object | semmle.label | getCredentials(...) : Object |
+| JwtNoVerifier.java:59:32:59:40 | JwtToken3 : String | semmle.label | JwtToken3 : String |
+| JwtNoVerifier.java:89:38:89:55 | token : String | semmle.label | token : String |
+| JwtNoVerifier.java:90:26:90:42 | decode(...) : DecodedJWT | semmle.label | decode(...) : DecodedJWT |
+| JwtNoVerifier.java:90:37:90:41 | token : String | semmle.label | token : String |
+| JwtNoVerifier.java:91:16:91:31 | of(...) : Optional [<element>] : DecodedJWT | semmle.label | of(...) : Optional [<element>] : DecodedJWT |
+| JwtNoVerifier.java:91:28:91:30 | jwt : DecodedJWT | semmle.label | jwt : DecodedJWT |
+| JwtNoVerifier.java:91:37:91:40 | item : DecodedJWT | semmle.label | item : DecodedJWT |
+| JwtNoVerifier.java:91:45:91:48 | item : DecodedJWT | semmle.label | item : DecodedJWT |
+| JwtNoVerifier.java:91:45:91:69 | getClaim(...) | semmle.label | getClaim(...) |
+subpaths

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

@@ -0,0 +1,2 @@
+query: experimental/Security/CWE/CWE-347/Auth0NoVerifier.ql
+postprocess: TestUtilities/PrettyPrintModels.ql