Java: JWT decoding without verification

[am0o0]

[JarLob]

We have already https://github.com/github/codeql/blob/main/java/ql/src/Security/CWE/CWE-347/MissingJWTSignatureCheck.ql (Which doesn't detect the CVE), but I'll leave it to the CodeQL team to decide if it is better to merge in this PR or later.

[am0o0]

@JarLob except for the tests that I don't know how to create the tests in java, I think everything is good now.

[JarLob]

@am0o0 Could you please mark this pull request as ready for review? Have you looked already how tests for java are made in https://github.com/github/codeql/tree/main/java/ql/test/query-tests/security/CWE-347?

[am0o0]

@JarLob Yes I tried to learn to create test cases by comparing them to a similar part of current query tests but I failed, I'll try again.

[JarLob]

@am0o0 We are sunsetting our codeql bug bounty program. To be eligible for the reward please work on the pull request ASAP.

[am0o0]

@JarLob I'm learning how to create stubs for Java, otherwise the tests are the same as the example file, so when I finish the work of creating the test for the other java PR I'll quickly create a test case here.

[am0o0]

@JarLob @owen-mc this PR is ready for review as I finalized the tests.

[smowton]

Do you need anything particular to servlet-api-4.0.1, or can you just use java/ql/test/stubs/servlet-api-2.4 instead of introducing new stubs?

[am0o0]

I tried it just now, and it failed:

codeql test run Auth0NoVerifier.qlref --show-extractor-output
Executing 1 tests in 1 directories.
Extracting test database in /home/am/CodeQL-home/codeql-repo-amammad/java/ql/test/experimental/query-tests/security/CWE-347.
[2024-07-18 20:59:59] [build-err] Exception in thread "main" Compilation errors were reported by javac.
[2024-07-18 20:59:59] [build-err]       at com.semmle.extractor.java.JavaExtractor.main(JavaExtractor.java:810)
[2024-07-18 20:59:59] [ERROR] Spawned process exited abnormally (code 1; tried to run: [/home/am/CodeQL-home/java/tools/linux/jdk-extractor-java/bin/java, -cp, /home/am/CodeQL-home/java/tools/semmle-extractor-java.jar:/home/am/CodeQL-home/java/tools/lombok-javac-extend.jar, -Xmx500m, com.semmle.extractor.java.JavaExtractor, --strict-javac-errors, --javac-args, -source, 8, -target, 8, -cp, /home/am/CodeQL-home/codeql-repo-amammad/java/ql/test/experimental/query-tests/security/CWE-347/../../../stubs/auth0-java-jwt-4.4.0:/home/am/CodeQL-home/codeql-repo-amammad/java/ql/test/experimental/query-tests/security/CWE-347/../../../../../test/stubs/servlet-api-2.4/, -encoding, UTF-8, JwtNoVerifier.java])
Could not extract a dataset in /home/am/CodeQL-home/codeql-repo-amammad/java/ql/test/experimental/query-tests/security/CWE-347: Extraction command /home/am/CodeQL-home/java/tools/linux/jdk-extractor-java/bin/java failed with status 1.
Extraction command /home/am/CodeQL-home/java/tools/linux/jdk-extractor-java/bin/java failed with status 1.
[1/1] FAILED(EXTRACTION) /home/am/CodeQL-home/codeql-repo-amammad/java/ql/test/experimental/query-tests/security/CWE-347/Auth0NoVerifier.qlref
Compiling queries in /home/am/CodeQL-home/codeql-repo-amammad/java/ql/test/experimental/query-tests/security/CWE-347.
Completed in 3.8s (extract 2.2s comp 0ms eval 0ms).
0 tests passed; 1 tests failed:
  FAILED: /home/am/CodeQL-home/codeql-repo-amammad/java/ql/test/experimental/query-tests/security/CWE-347/Auth0NoVerifier.qlref

[owen-mc]

@am0o0 Can you be more specific about what you tried?

[am0o0]

I simply replaced my servlet api stub with "java/ql/test/stubs/servlet-api-2.4" in the options file, I double-checked that the new path of the stub was correct too.

[am0o0]

it is a very old version i dont expect to work honestly. Also we can think in this way that we have the stubs for new version now.😅

[smowton]

You can see what the extractions are in CWE-347/CWE-347.testproj/log/javac-output-*.log -- in this case it was simply that WebServlet is new, and getWriter throws IOException in Servlet 2.5. I have taken the liberty of creating https://github.com/github/codeql/pull/17020 to fix that and polish this PR.

[smowton]

So, looking at this I suggest this could be simplified:

All you're trying to do is notice when the result of decode flows to getClaim or similar ("get-payload"), but does NOT flow to a verify method.

To do this you only need one dataflow config, and you don't need flow states.

Suggest simply:

Source = result of the decode method Sink = either a verify or a get-payload method No additional flow steps needed

Then the query result is simply those (source, sink) pairs where the sink is a get-payload method and there does not exist a (source2, sink2) such that sink2 is a verify method and source.getNode = source2.getNode.

If it should turn out there are false positives due to decoding JWTs that don't come from a remote source, then flow-state can be used to track RemoteFlowSource -> decode -> verify/get-payload -- in that case decode is an additional flow step, but you still don't need flow-state because the type system guarantees that if we're verifying something from a remote flow source, it must have been decoded at some point.

Finally I would remove the flow sources that seem to regard any string-typed variable initializer as a source. In that case you might as well call any string at all a source (what's special about initializing a variable?), and in that case you might as well just start tracking flow from the decode method instead, since clearly it got a string input from somewhere.

[smowton]

Other simple things to fix:

MethodAccess -> MethodCall

DataFlow::FlowState -> string (but as mentioned above this should be removed anyway)

Change the query id and description to reflect this being an auth0-specific query (and not clash with the non-experimental query that also checks for JWT mistakes)

[am0o0]

@smowton thank you so much for your thorough review!

[github-actions[bot]]

QHelp previews:

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

Missing JWT signature check

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.

Recommendation

Don't use information from a JWT without verifying that JWT.

Example

The following example illustrates secure and insecure use of the Auth0 `java-jwt` library.

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("");
  }

}

References

• The incorrect use of JWT in ShenyuAdminBootstrap allows an attacker to bypass authentication.
• Common Weakness Enumeration: CWE-347.

[am0o0]

@JarLob Could you please upload the DB, I lost it and I can't create it again because of some errors :)

[am0o0]

Hi, I tried to fix my mistake with minimum changes, you can check the changes with commits, I tried to have a good commit history for this to make the review easier.

[am0o0]

@JarLob the CVE can be detected with the local source version of the final query now. Also I managed to create the DB.

[JarLob]

Hi, it still doesn't work. Here is the database https://we.tl/t-WOpOJykqkl

On Wed, Jul 31, 2024 at 11:07 AM Am @.***> wrote:

[image: am0o0]am0o0 left a comment (github/codeql#14089) https://github.com/github/codeql/pull/14089#issuecomment-2260024804

@JarLob https://github.com/JarLob the CVE can be detected with the local source version of the final query now. Also I managed to create the DB.

— Reply to this email directly, view it on GitHub https://github.com/github/codeql/pull/14089#issuecomment-2260024804, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGLK53EU4HOXZICURTKYW3DZPCSMZAVCNFSM6AAAAAA4C4SPJCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRQGAZDIOBQGQ . You are receiving this because you were mentioned.Message ID: @.***>

[am0o0]

@JarLob I think I'm wrong maybe, could you confirm that this was the sink :) ?

[JarLob]

Maybe my codeql branch is outdated. I'll check out the pr branch and try again.

On Wed, Jul 31, 2024 at 2:11 PM Am @.***> wrote:

[image: am0o0]am0o0 left a comment (github/codeql#14089) https://github.com/github/codeql/pull/14089#issuecomment-2260374271

@JarLob https://github.com/JarLob I think I'm wrong maybe, could you confirm that this was the sink :) ? image.png (view on web) https://github.com/user-attachments/assets/aae8b2c1-4a33-4192-bf9d-d77a35f9a659

— Reply to this email directly, view it on GitHub https://github.com/github/codeql/pull/14089#issuecomment-2260374271, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGLK53GYOEPL5HSKBJRV2VDZPDH5PAVCNFSM6AAAAAA4C4SPJCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRQGM3TIMRXGE . You are receiving this because you were mentioned.Message ID: @.***>

[JarLob]

I have opened your branch in a codespace and got 0 results:

On Wed, Jul 31, 2024 at 2:20 PM Jaroslav Lobacevski @.***> wrote:

Maybe my codeql branch is outdated. I'll check out the pr branch and try again.

On Wed, Jul 31, 2024 at 2:11 PM Am @.***> wrote:

[image: am0o0]am0o0 left a comment (github/codeql#14089) https://github.com/github/codeql/pull/14089#issuecomment-2260374271

@JarLob https://github.com/JarLob I think I'm wrong maybe, could you confirm that this was the sink :) ? image.png (view on web) https://github.com/user-attachments/assets/aae8b2c1-4a33-4192-bf9d-d77a35f9a659

— Reply to this email directly, view it on GitHub https://github.com/github/codeql/pull/14089#issuecomment-2260374271, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGLK53GYOEPL5HSKBJRV2VDZPDH5PAVCNFSM6AAAAAA4C4SPJCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRQGM3TIMRXGE . You are receiving this because you were mentioned.Message ID: @.***>

[JarLob]

Ah, I missed that local.ql. I can only blame GitHub UI for cutting the file list :) It gives the result.

Message ID: @.***>

[smowton]

@jarlob are you able to confirm this latest draft is satisfactory from your point of view?

[JarLob]

Yes, it is.