From fa6ac063d1cb75aed1586f8526426e6aa346fbbb Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Mon, 30 Jan 2023 14:18:04 -0500 Subject: [PATCH 1/7] Add com.auth0.jwt.algorithm.Algorithm sinks The HMAC* constructors of the com.auth0.jwt.algorithm.Algorithm class take a secret as a parameter. Therefore, the arguments should be added to be checked for hardcoded credentials. --- .../code/java/security/SensitiveApi.qll | 3 + .../security/CWE-321/HardcodedJwtKey.java | 4 +- .../CWE-798/semmle/tests/HardcodedJwtKey.java | 65 +++++++++++++++++++ .../security/CWE-798/semmle/tests/options | 2 +- 4 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedJwtKey.java diff --git a/java/ql/lib/semmle/code/java/security/SensitiveApi.qll b/java/ql/lib/semmle/code/java/security/SensitiveApi.qll index 9936ab202a34..a1c93d3a0d92 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveApi.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveApi.qll @@ -490,5 +490,8 @@ 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;HMAC384(String);0", + "com.auth0.jwt.algorithms.Algorithm;HMAC512(String);0" ] } diff --git a/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java b/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java index cfcb027755bf..85e077d123aa 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java +++ b/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java @@ -22,7 +22,7 @@ public String accessTokenBad(String username) { .withExpiresAt(new Date(new Date().getTime() + ACCESS_EXPIRE_TIME)) .withIssuer(ISSUER) .withClaim("username", username) - .sign(algorithm); + .sign(algorithm); // $ HardcodedCredentialsApiCall } // GOOD: Get secret from system configuration then sign a token @@ -43,7 +43,7 @@ public boolean verifyTokenBad(String token) { .withIssuer(ISSUER) .build(); try { - verifier.verify(token); + verifier.verify(token); // $ HardcodedCredentialsApiCall return true; } catch (JWTVerificationException e) { return false; diff --git a/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedJwtKey.java b/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedJwtKey.java new file mode 100644 index 000000000000..f2231aaecb72 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedJwtKey.java @@ -0,0 +1,65 @@ +import java.util.Date; +import java.util.Properties; + +import com.auth0.jwt.JWT; +import com.auth0.jwt.algorithms.Algorithm; +import com.auth0.jwt.exceptions.JWTVerificationException; +import com.auth0.jwt.interfaces.JWTVerifier; + +public class HardcodedJwtKey { + // 15 minutes + private static final long ACCESS_EXPIRE_TIME = 1000 * 60 * 15; + + private static final String ISSUER = "example_com"; + + private static final String SECRET = "hardcoded_secret"; + + // BAD: Get secret from hardcoded string then sign a JWT token + public String accessTokenBad(String username) { + Algorithm algorithm = Algorithm.HMAC256(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 accessTokenGood(String username) { + String tokenSecret = System.getenv("SECRET_KEY"); + Algorithm algorithm = Algorithm.HMAC256(tokenSecret); + + return JWT.create() + .withExpiresAt(new Date(new Date().getTime() + ACCESS_EXPIRE_TIME)) + .withIssuer(ISSUER) + .withClaim("username", username) + .sign(algorithm); + } + + // BAD: Get secret from hardcoded string then verify a JWT token + public boolean verifyTokenBad(String token) { + JWTVerifier verifier = JWT.require(Algorithm.HMAC256(SECRET)) // $ HardcodedCredentialsApiCall + .withIssuer(ISSUER) + .build(); + try { + verifier.verify(token); + return true; + } catch (JWTVerificationException e) { + return false; + } + } + + // GOOD: Get secret from environment variable then verify a JWT token + public boolean verifyTokenGood(String token) { + JWTVerifier verifier = JWT.require(Algorithm.HMAC256(System.getenv("SECRET_KEY"))) + .withIssuer(ISSUER) + .build(); + try { + verifier.verify(token); + return true; + } catch (JWTVerificationException e) { + return false; + } + } +} 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 From d71386e001257c7970ba19fc686145a5484f0808 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Mon, 30 Jan 2023 14:22:19 -0500 Subject: [PATCH 2/7] Add example file for documentation --- .../Security/CWE/CWE-798/HardcodedJwtKey.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 java/ql/src/Security/CWE/CWE-798/HardcodedJwtKey.java diff --git a/java/ql/src/Security/CWE/CWE-798/HardcodedJwtKey.java b/java/ql/src/Security/CWE/CWE-798/HardcodedJwtKey.java new file mode 100644 index 000000000000..697127946895 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-798/HardcodedJwtKey.java @@ -0,0 +1,26 @@ +// 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); From 3ff1a97e38cbe9f336bd8cc0c07c43dc639b1142 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 14 Feb 2023 11:50:18 -0500 Subject: [PATCH 3/7] Add `byte[]` signatures --- java/ql/lib/semmle/code/java/security/SensitiveApi.qll | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/java/ql/lib/semmle/code/java/security/SensitiveApi.qll b/java/ql/lib/semmle/code/java/security/SensitiveApi.qll index a1c93d3a0d92..1baff16a100b 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveApi.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveApi.qll @@ -491,7 +491,10 @@ private predicate otherApiCallableCredentialParam(string s) { "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;HMAC512(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" ] } From 6de946ef00e79660e8629ec36b484ae510e5f4a5 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 17 Feb 2023 09:35:38 -0500 Subject: [PATCH 4/7] Remove experimental files --- .../Security/CWE/CWE-321/HardcodedJwtKey.java | 26 ---- .../CWE/CWE-321/HardcodedJwtKey.qhelp | 46 ------ .../Security/CWE/CWE-321/HardcodedJwtKey.ql | 20 --- .../Security/CWE/CWE-321/HardcodedJwtKey.qll | 131 ------------------ .../security/CWE-321/HardcodedJwtKey.expected | 25 ---- .../security/CWE-321/HardcodedJwtKey.java | 65 --------- .../security/CWE-321/HardcodedJwtKey.qlref | 1 - .../query-tests/security/CWE-321/options | 1 - 8 files changed, 315 deletions(-) delete mode 100644 java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.java delete mode 100644 java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.qhelp delete mode 100644 java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.ql delete mode 100644 java/ql/src/experimental/Security/CWE/CWE-321/HardcodedJwtKey.qll delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.expected delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.qlref delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-321/options 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.java b/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java deleted file mode 100644 index 85e077d123aa..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java +++ /dev/null @@ -1,65 +0,0 @@ -import java.util.Date; -import java.util.Properties; - -import com.auth0.jwt.JWT; -import com.auth0.jwt.algorithms.Algorithm; -import com.auth0.jwt.exceptions.JWTVerificationException; -import com.auth0.jwt.interfaces.JWTVerifier; - -public class HardcodedJwtKey { - // 15 minutes - private static final long ACCESS_EXPIRE_TIME = 1000 * 60 * 15; - - private static final String ISSUER = "example_com"; - - private static final String SECRET = "hardcoded_secret"; - - // BAD: Get secret from hardcoded string then sign a JWT token - public String accessTokenBad(String username) { - Algorithm algorithm = Algorithm.HMAC256(SECRET); - - return JWT.create() - .withExpiresAt(new Date(new Date().getTime() + ACCESS_EXPIRE_TIME)) - .withIssuer(ISSUER) - .withClaim("username", username) - .sign(algorithm); // $ HardcodedCredentialsApiCall - } - - // GOOD: Get secret from system configuration then sign a token - public String accessTokenGood(String username) { - String tokenSecret = System.getenv("SECRET_KEY"); - Algorithm algorithm = Algorithm.HMAC256(tokenSecret); - - return JWT.create() - .withExpiresAt(new Date(new Date().getTime() + ACCESS_EXPIRE_TIME)) - .withIssuer(ISSUER) - .withClaim("username", username) - .sign(algorithm); - } - - // BAD: Get secret from hardcoded string then verify a JWT token - public boolean verifyTokenBad(String token) { - JWTVerifier verifier = JWT.require(Algorithm.HMAC256(SECRET)) - .withIssuer(ISSUER) - .build(); - try { - verifier.verify(token); // $ HardcodedCredentialsApiCall - return true; - } catch (JWTVerificationException e) { - return false; - } - } - - // GOOD: Get secret from environment variable then verify a JWT token - public boolean verifyTokenGood(String token) { - JWTVerifier verifier = JWT.require(Algorithm.HMAC256(System.getenv("SECRET_KEY"))) - .withIssuer(ISSUER) - .build(); - try { - verifier.verify(token); - return true; - } catch (JWTVerificationException e) { - return false; - } - } -} 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 From 4aec708fac4f1b747308851a8fb6761f29deea10 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 17 Feb 2023 10:02:32 -0500 Subject: [PATCH 5/7] Add change note --- .../2023-02-17-add-hardcoded-secret-for-jwt-tokens.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 java/ql/lib/change-notes/2023-02-17-add-hardcoded-secret-for-jwt-tokens.md 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). From 06a1368e7c4e854db4790934e2a3b31b2a6af294 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Fri, 17 Feb 2023 10:18:41 -0500 Subject: [PATCH 6/7] Additional test cases --- .../CWE-798/semmle/tests/HardcodedJwtKey.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedJwtKey.java b/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedJwtKey.java index f2231aaecb72..c4b0b1914eb1 100644 --- a/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedJwtKey.java +++ b/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedJwtKey.java @@ -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); + } + } From ed1aac1aa5b93a53da5c8f227a684b84cf17deed Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Tue, 21 Feb 2023 10:54:32 -0500 Subject: [PATCH 7/7] Remove unneeded example file --- .../Security/CWE/CWE-798/HardcodedJwtKey.java | 26 ------------------- 1 file changed, 26 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-798/HardcodedJwtKey.java diff --git a/java/ql/src/Security/CWE/CWE-798/HardcodedJwtKey.java b/java/ql/src/Security/CWE/CWE-798/HardcodedJwtKey.java deleted file mode 100644 index 697127946895..000000000000 --- a/java/ql/src/Security/CWE/CWE-798/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);