diff --git a/java/ql/lib/change-notes/2023-02-17-add-hardcoded-secret-for-jwt-tokens.md b/java/ql/lib/change-notes/2023-02-17-add-hardcoded-secret-for-jwt-tokens.md new file mode 100644 index 000000000000..408bb13755b9 --- /dev/null +++ b/java/ql/lib/change-notes/2023-02-17-add-hardcoded-secret-for-jwt-tokens.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added new sinks for `java/hardcoded-credential-api-call` to identify the use of hardcoded secrets in the creation and verification of JWT tokens using `com.auth0.jwt`. These sinks are from [an experimental query submitted by @luchua](https://github.com/github/codeql/pull/9036). diff --git a/java/ql/lib/semmle/code/java/security/SensitiveApi.qll b/java/ql/lib/semmle/code/java/security/SensitiveApi.qll index 9936ab202a34..1baff16a100b 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveApi.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveApi.qll @@ -490,5 +490,11 @@ private predicate otherApiCallableCredentialParam(string s) { "com.microsoft.sqlserver.jdbc.SQLServerDataSource;setPassword(String);0", "com.microsoft.sqlserver.jdbc.SQLServerDataSource;getConnection(String, String);0", "com.microsoft.sqlserver.jdbc.SQLServerDataSource;getConnection(String, String);1", + "com.auth0.jwt.algorithms.Algorithm;HMAC256(String);0", + "com.auth0.jwt.algorithms.Algorithm;HMAC256(byte[]);0", + "com.auth0.jwt.algorithms.Algorithm;HMAC384(String);0", + "com.auth0.jwt.algorithms.Algorithm;HMAC384(byte[]);0", + "com.auth0.jwt.algorithms.Algorithm;HMAC512(String);0", + "com.auth0.jwt.algorithms.Algorithm;HMAC512(byte[]);0" ] } diff --git a/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.java b/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.java deleted file mode 100644 index 697127946895..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.java +++ /dev/null @@ -1,26 +0,0 @@ -// BAD: Get secret from hardcoded string then sign a JWT token -Algorithm algorithm = Algorithm.HMAC256("hardcoded_secret"); -JWT.create() - .withClaim("username", username) - .sign(algorithm); -} - -// BAD: Get secret from hardcoded string then verify a JWT token -JWTVerifier verifier = JWT.require(Algorithm.HMAC256("hardcoded_secret")) - .withIssuer(ISSUER) - .build(); -verifier.verify(token); - -// GOOD: Get secret from system configuration then sign a token -String tokenSecret = System.getenv("SECRET_KEY"); -Algorithm algorithm = Algorithm.HMAC256(tokenSecret); -JWT.create() - .withClaim("username", username) - .sign(algorithm); - } - -// GOOD: Get secret from environment variable then verify a JWT token -JWTVerifier verifier = JWT.require(Algorithm.HMAC256(System.getenv("SECRET_KEY"))) - .withIssuer(ISSUER) - .build(); -verifier.verify(token); diff --git a/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.qhelp b/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.qhelp deleted file mode 100644 index e6a25a3b96c7..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.qhelp +++ /dev/null @@ -1,46 +0,0 @@ - - - -

- JWT (JSON Web Token) is an open standard (RFC 7519) that defines a way to provide information - within a JSON object between two parties. JWT is widely used for sharing security information - between two parties in web applications. Each JWT contains encoded JSON objects, including a - set of claims. JWTs are signed using a cryptographic algorithm to ensure that the claims cannot - be altered after the token is issued. -

-

- The most basic mistake is using hardcoded secrets for JWT generation/verification. This allows - an attacker to forge the token if the source code (and JWT secret in it) is publicly exposed or - leaked, which leads to authentication bypass or privilege escalation. -

- - - -

- Generating a cryptographically secure secret key during application initialization and using this - generated key for JWT signing/verification requests can prevent this vulnerability. Or safely store - the secret key in a key vault that cannot be leaked in source code. -

- - - -

- The following examples show the bad case and the good case respectively. The bad - methods show a hardcoded secret key is used to sign and verify JWT tokens. In the good - method, the secret key is loaded from a system environment during application initialization. -

- - - - -
  • - Semgrep Blog: - Hardcoded secrets, unverified tokens, and other common JWT mistakes -
    • -
  • - CVE-2022-24860: - Databasir 1.01 has Use of Hard-coded Cryptographic Key vulnerability. -
    • -     - -     diff --git a/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.ql b/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.ql deleted file mode 100644 index 521a355bdab4..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.ql +++ /dev/null @@ -1,20 +0,0 @@ -/** - * @name Use of a hardcoded key for signing JWT - * @description Using a hardcoded key for signing JWT can allow an attacker to compromise security. - * @kind path-problem - * @problem.severity error - * @id java/hardcoded-jwt-key - * @tags security - * experimental - * external/cwe/cwe-321 - */ - -import java -import HardcodedJwtKey -import semmle.code.java.dataflow.TaintTracking -import DataFlow::PathGraph - -from DataFlow::PathNode source, DataFlow::PathNode sink, HardcodedJwtKeyConfiguration cfg -where cfg.hasFlowPath(source, sink) -select sink.getNode(), source, sink, "$@ is used to sign a JWT token.", source.getNode(), - "Hardcoded String" diff --git a/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.qll b/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.qll deleted file mode 100644 index 09db11dd40be..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.qll +++ /dev/null @@ -1,131 +0,0 @@ -/** - * Provides sources and sinks for detecting JWT token signing vulnerabilities. - */ - -import java -private import semmle.code.java.dataflow.ExternalFlow -private import semmle.code.java.dataflow.FlowSources - -private class ActivateModels extends ActiveExperimentalModels { - ActivateModels() { this = "hardcoded-jwt-key" } -} - -/** The class `com.auth0.jwt.JWT`. */ -class Jwt extends RefType { - Jwt() { this.hasQualifiedName("com.auth0.jwt", "JWT") } -} - -/** The class `com.auth0.jwt.JWTCreator.Builder`. */ -class JwtBuilder extends RefType { - JwtBuilder() { this.hasQualifiedName("com.auth0.jwt", "JWTCreator$Builder") } -} - -/** The class `com.auth0.jwt.algorithms.Algorithm`. */ -class JwtAlgorithm extends RefType { - JwtAlgorithm() { this.hasQualifiedName("com.auth0.jwt.algorithms", "Algorithm") } -} - -/** - * The interface `com.auth0.jwt.interfaces.JWTVerifier` or its implementation - * `com.auth0.jwt.JWTVerifier`. - */ -class JwtVerifier extends RefType { - JwtVerifier() { - this.hasQualifiedName(["com.auth0.jwt", "com.auth0.jwt.interfaces"], "JWTVerifier") - } -} - -/** A method that creates an instance of `com.auth0.jwt.algorithms.Algorithm`. */ -class GetAlgorithmMethod extends Method { - GetAlgorithmMethod() { - this.getDeclaringType() instanceof JwtAlgorithm and - this.getName().matches(["HMAC%", "ECDSA%", "RSA%"]) - } -} - -/** The `require` method of `com.auth0.jwt.JWT`. */ -class RequireMethod extends Method { - RequireMethod() { - this.getDeclaringType() instanceof Jwt and - this.hasName("require") - } -} - -/** The `sign` method of `com.auth0.jwt.JWTCreator.Builder`. */ -class SignTokenMethod extends Method { - SignTokenMethod() { - this.getDeclaringType() instanceof JwtBuilder and - this.hasName("sign") - } -} - -/** The `verify` method of `com.auth0.jwt.interfaces.JWTVerifier`. */ -class VerifyTokenMethod extends Method { - VerifyTokenMethod() { - this.getDeclaringType() instanceof JwtVerifier and - this.hasName("verify") - } -} - -/** - * A data flow source for JWT token signing vulnerabilities. - */ -abstract class JwtKeySource extends DataFlow::Node { } - -/** - * A data flow sink for JWT token signing vulnerabilities. - */ -abstract class JwtTokenSink extends DataFlow::Node { } - -/** - * A hardcoded string literal as a source for JWT token signing vulnerabilities. - */ -class HardcodedKeyStringSource extends JwtKeySource { - HardcodedKeyStringSource() { exists(this.asExpr().(CompileTimeConstantExpr).getStringValue()) } -} - -/** - * An expression used to sign JWT tokens as a sink of JWT token signing vulnerabilities. - */ -private class SignTokenSink extends JwtTokenSink { - SignTokenSink() { - exists(MethodAccess ma | - ma.getMethod() instanceof SignTokenMethod and - this.asExpr() = ma.getArgument(0) - ) - } -} - -/** - * An expression used to verify JWT tokens as a sink of JWT token signing vulnerabilities. - */ -private class VerifyTokenSink extends JwtTokenSink { - VerifyTokenSink() { - exists(MethodAccess ma | - ma.getMethod() instanceof VerifyTokenMethod and - this.asExpr() = ma.getQualifier() - ) - } -} - -/** - * A configuration depicting taint flow for checking JWT token signing vulnerabilities. - */ -class HardcodedJwtKeyConfiguration extends TaintTracking::Configuration { - HardcodedJwtKeyConfiguration() { this = "Hard-coded JWT Signing Key" } - - override predicate isSource(DataFlow::Node source) { source instanceof JwtKeySource } - - override predicate isSink(DataFlow::Node sink) { sink instanceof JwtTokenSink } - - override predicate isAdditionalTaintStep(DataFlow::Node prev, DataFlow::Node succ) { - exists(MethodAccess ma | - ( - ma.getMethod() instanceof GetAlgorithmMethod or - ma.getMethod() instanceof RequireMethod - ) and - prev.asExpr() = ma.getArgument(0) and - succ.asExpr() = ma - ) - } -} diff --git a/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.expected b/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.expected deleted file mode 100644 index 15e882db4cdd..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.expected +++ /dev/null @@ -1,25 +0,0 @@ -edges -| HardcodedJwtKey.java:15:33:15:38 | SECRET : String | HardcodedJwtKey.java:19:49:19:54 | SECRET : String | -| HardcodedJwtKey.java:15:33:15:38 | SECRET : String | HardcodedJwtKey.java:42:62:42:67 | SECRET : String | -| HardcodedJwtKey.java:15:42:15:59 | "hardcoded_secret" : String | HardcodedJwtKey.java:15:33:15:38 | SECRET : String | -| HardcodedJwtKey.java:19:49:19:54 | SECRET : String | HardcodedJwtKey.java:25:23:25:31 | algorithm | -| HardcodedJwtKey.java:42:32:42:69 | require(...) : Verification | HardcodedJwtKey.java:42:32:43:35 | withIssuer(...) : Verification | -| HardcodedJwtKey.java:42:32:43:35 | withIssuer(...) : Verification | HardcodedJwtKey.java:42:32:44:24 | build(...) : JWTVerifier | -| HardcodedJwtKey.java:42:32:44:24 | build(...) : JWTVerifier | HardcodedJwtKey.java:46:13:46:20 | verifier | -| HardcodedJwtKey.java:42:62:42:67 | SECRET : String | HardcodedJwtKey.java:42:32:42:69 | require(...) : Verification | -nodes -| HardcodedJwtKey.java:15:33:15:38 | SECRET : String | semmle.label | SECRET : String | -| HardcodedJwtKey.java:15:42:15:59 | "hardcoded_secret" : String | semmle.label | "hardcoded_secret" : String | -| HardcodedJwtKey.java:19:49:19:54 | SECRET : String | semmle.label | SECRET : String | -| HardcodedJwtKey.java:25:23:25:31 | algorithm | semmle.label | algorithm | -| HardcodedJwtKey.java:42:32:42:69 | require(...) : Verification | semmle.label | require(...) : Verification | -| HardcodedJwtKey.java:42:32:43:35 | withIssuer(...) : Verification | semmle.label | withIssuer(...) : Verification | -| HardcodedJwtKey.java:42:32:44:24 | build(...) : JWTVerifier | semmle.label | build(...) : JWTVerifier | -| HardcodedJwtKey.java:42:62:42:67 | SECRET : String | semmle.label | SECRET : String | -| HardcodedJwtKey.java:46:13:46:20 | verifier | semmle.label | verifier | -subpaths -#select -| HardcodedJwtKey.java:25:23:25:31 | algorithm | HardcodedJwtKey.java:15:42:15:59 | "hardcoded_secret" : String | HardcodedJwtKey.java:25:23:25:31 | algorithm | $@ is used to sign a JWT token. | HardcodedJwtKey.java:15:42:15:59 | "hardcoded_secret" | Hardcoded String | -| HardcodedJwtKey.java:25:23:25:31 | algorithm | HardcodedJwtKey.java:19:49:19:54 | SECRET : String | HardcodedJwtKey.java:25:23:25:31 | algorithm | $@ is used to sign a JWT token. | HardcodedJwtKey.java:19:49:19:54 | SECRET | Hardcoded String | -| HardcodedJwtKey.java:46:13:46:20 | verifier | HardcodedJwtKey.java:15:42:15:59 | "hardcoded_secret" : String | HardcodedJwtKey.java:46:13:46:20 | verifier | $@ is used to sign a JWT token. | HardcodedJwtKey.java:15:42:15:59 | "hardcoded_secret" | Hardcoded String | -| HardcodedJwtKey.java:46:13:46:20 | verifier | HardcodedJwtKey.java:42:62:42:67 | SECRET : String | HardcodedJwtKey.java:46:13:46:20 | verifier | $@ is used to sign a JWT token. | HardcodedJwtKey.java:42:62:42:67 | SECRET | Hardcoded String | diff --git a/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.qlref b/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.qlref deleted file mode 100644 index 3da970cd3805..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE/CWE-321/HardcodedJwtKey.ql \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-321/options b/java/ql/test/experimental/query-tests/security/CWE-321/options deleted file mode 100644 index ab6ca411a02d..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-321/options +++ /dev/null @@ -1 +0,0 @@ -//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/auth0-jwt-2.3 diff --git a/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java b/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedJwtKey.java similarity index 53% rename from java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java rename to java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedJwtKey.java index cfcb027755bf..c4b0b1914eb1 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java +++ b/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedJwtKey.java @@ -16,7 +16,7 @@ public class HardcodedJwtKey { // BAD: Get secret from hardcoded string then sign a JWT token public String accessTokenBad(String username) { - Algorithm algorithm = Algorithm.HMAC256(SECRET); + Algorithm algorithm = Algorithm.HMAC256(SECRET); // $ HardcodedCredentialsApiCall return JWT.create() .withExpiresAt(new Date(new Date().getTime() + ACCESS_EXPIRE_TIME)) @@ -39,7 +39,7 @@ public String accessTokenGood(String username) { // BAD: Get secret from hardcoded string then verify a JWT token public boolean verifyTokenBad(String token) { - JWTVerifier verifier = JWT.require(Algorithm.HMAC256(SECRET)) + JWTVerifier verifier = JWT.require(Algorithm.HMAC256(SECRET)) // $ HardcodedCredentialsApiCall .withIssuer(ISSUER) .build(); try { @@ -62,4 +62,49 @@ public boolean verifyTokenGood(String token) { return false; } } + + public String accessTokenBad384(String username) { + Algorithm algorithm = Algorithm.HMAC384(SECRET); // $ HardcodedCredentialsApiCall + + return JWT.create() + .withExpiresAt(new Date(new Date().getTime() + ACCESS_EXPIRE_TIME)) + .withIssuer(ISSUER) + .withClaim("username", username) + .sign(algorithm); + } + + // GOOD: Get secret from system configuration then sign a token + public String accessTokenGood384(String username) { + String tokenSecret = System.getenv("SECRET_KEY"); + Algorithm algorithm = Algorithm.HMAC384(tokenSecret); + + return JWT.create() + .withExpiresAt(new Date(new Date().getTime() + ACCESS_EXPIRE_TIME)) + .withIssuer(ISSUER) + .withClaim("username", username) + .sign(algorithm); + } + + public String accessTokenBad512(String username) { + Algorithm algorithm = Algorithm.HMAC512(SECRET); // $ HardcodedCredentialsApiCall + + return JWT.create() + .withExpiresAt(new Date(new Date().getTime() + ACCESS_EXPIRE_TIME)) + .withIssuer(ISSUER) + .withClaim("username", username) + .sign(algorithm); + } + + // GOOD: Get secret from system configuration then sign a token + public String accessTokenGood512(String username) { + String tokenSecret = System.getenv("SECRET_KEY"); + Algorithm algorithm = Algorithm.HMAC512(tokenSecret); + + return JWT.create() + .withExpiresAt(new Date(new Date().getTime() + ACCESS_EXPIRE_TIME)) + .withIssuer(ISSUER) + .withClaim("username", username) + .sign(algorithm); + } + } diff --git a/java/ql/test/query-tests/security/CWE-798/semmle/tests/options b/java/ql/test/query-tests/security/CWE-798/semmle/tests/options index 215e0929c439..18ff7ebd8a93 100644 --- a/java/ql/test/query-tests/security/CWE-798/semmle/tests/options +++ b/java/ql/test/query-tests/security/CWE-798/semmle/tests/options @@ -1 +1 @@ -// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/amazon-aws-sdk-1.11.700:${testdir}/../../../../../stubs/azure-sdk-for-java:${testdir}/../../../../../stubs/shiro-core-1.4.0:${testdir}/../../../../../stubs/jsch-0.1.55:${testdir}/../../../../../stubs/ganymed-ssh-2-260:${testdir}/../../../../../stubs/apache-mina-sshd-2.8.0:${testdir}/../../../../../stubs/sshj-0.33.0:${testdir}/../../../../../stubs/j2ssh-1.5.5:${testdir}/../../../../../stubs/trilead-ssh2-212:${testdir}/../../../../../stubs/apache-commons-net-3.8.0:${testdir}/../../../../../stubs/mongodbClient:${testdir}/../../../../../stubs/mssql-jdbc-12.2.0 +// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/amazon-aws-sdk-1.11.700:${testdir}/../../../../../stubs/azure-sdk-for-java:${testdir}/../../../../../stubs/shiro-core-1.4.0:${testdir}/../../../../../stubs/jsch-0.1.55:${testdir}/../../../../../stubs/ganymed-ssh-2-260:${testdir}/../../../../../stubs/apache-mina-sshd-2.8.0:${testdir}/../../../../../stubs/sshj-0.33.0:${testdir}/../../../../../stubs/j2ssh-1.5.5:${testdir}/../../../../../stubs/trilead-ssh2-212:${testdir}/../../../../../stubs/apache-commons-net-3.8.0:${testdir}/../../../../../stubs/mongodbClient:${testdir}/../../../../../stubs/mssql-jdbc-12.2.0:${testdir}/../../../../../stubs/auth0-jwt-2.3