Java: Promote Missing JWT signature check query from experimental

java/change-notes/2021-05-17-missing-jwt-signature-check-query.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The query "Missing JWT signature check" (`java/missing-jwt-signature-check`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @intrigus-lgtm](https://github.com/github/codeql/pull/5597).

java/ql/src/experimental/Security/CWE/CWE-347/MissingJWTSignatureCheck.qhelp → java/ql/src/Security/CWE/CWE-347/MissingJWTSignatureCheck.qhelp

@@ -25,9 +25,9 @@ by overriding the <code>onPlaintextJws</code> or <code>onClaimsJws</code> of <co
 <example>
 
 <p>The following example shows four cases where a signing key is set for a parser.
-In the first bad case the <code>parse</code> method is used which will not validate the signature.
-The second bad case uses a <code>JwtHandlerAdapter</code> where the <code>onPlaintextJwt</code> method is overriden so it will not validate the signature.
-The third and fourth good cases use <code>parseClaimsJws</code> method or override the <code>onPlaintextJws</code> method.
+In the first 'BAD' case the <code>parse</code> method is used, which will not validate the signature.
+The second 'BAD' case uses a <code>JwtHandlerAdapter</code> where the <code>onPlaintextJwt</code> method is overriden, so it will not validate the signature.
+The third and fourth 'GOOD' cases use <code>parseClaimsJws</code> method or override the <code>onPlaintextJws</code> method.
 </p>
 
 <sample src="MissingJWTSignatureCheck.java" />

java/ql/src/Security/CWE/CWE-347/MissingJWTSignatureCheck.ql

@@ -0,0 +1,20 @@
+/**
+ * @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.security.MissingJWTSignatureCheckQuery
+import DataFlow::PathGraph
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, MissingJwtSignatureCheckConf conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "A signing key is set $@, but the signature is not verified.",
+  source.getNode(), "here"

java/ql/src/semmle/code/java/security/JWT.qll

@@ -0,0 +1,127 @@
+/** Provides classes for working with JSON Web Token (JWT) libraries. */
+
+import java
+private import semmle.code.java.dataflow.DataFlow
+
+/** A method access that assigns signing keys to a JWT parser. */
+class JwtParserWithInsecureParseSource extends DataFlow::Node {
+  JwtParserWithInsecureParseSource() {
+    exists(MethodAccess ma, Method m |
+      m.getDeclaringType().getASupertype*() instanceof TypeJwtParser or
+      m.getDeclaringType().getASupertype*() instanceof TypeJwtParserBuilder
+    |
+      this.asExpr() = ma and
+      ma.getMethod() = m and
+      m.hasName(["setSigningKey", "setSigningKeyResolver"])
+    )
+  }
+}
+
+/**
+ * The qualifier of an insecure parsing method.
+ * That is, either the qualifier of a call to the `parse(token)`,
+ * `parseClaimsJwt(token)` or `parsePlaintextJwt(token)` methods or
+ * the qualifier of a call to a `parse(token, handler)` method
+ * where the `handler` is considered insecure.
+ */
+class JwtParserWithInsecureParseSink extends DataFlow::Node {
+  MethodAccess insecureParseMa;
+
+  JwtParserWithInsecureParseSink() {
+    insecureParseMa.getQualifier() = this.asExpr() and
+    exists(Method m |
+      insecureParseMa.getMethod() = m and
+      m.getDeclaringType().getASupertype*() instanceof TypeJwtParser and
+      m.hasName(["parse", "parseClaimsJwt", "parsePlaintextJwt"]) and
+      (
+        m.getNumberOfParameters() = 1
+        or
+        isInsecureJwtHandler(insecureParseMa.getArgument(1))
+      )
+    )
+  }
+
+  /** Gets the method access that does the insecure parsing. */
+  MethodAccess getParseMethodAccess() { result = insecureParseMa }
+}
+
+/**
+ * A unit class for adding additional flow steps.
+ *
+ * Extend this class to add additional flow steps that should apply to the `MissingJwtSignatureCheckConf`.
+ */
+class JwtParserWithInsecureParseAdditionalFlowStep extends Unit {
+  abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
+}
+
+/** A set of additional flow steps to consider when working with JWT parsing related data flows. */
+private class DefaultJwtParserWithInsecureParseAdditionalFlowStep extends JwtParserWithInsecureParseAdditionalFlowStep {
+  override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
+    jwtParserStep(node1.asExpr(), node2.asExpr())
+  }
+}
+
+/** Models the builder style of `JwtParser` and `JwtParserBuilder`. */
+private predicate jwtParserStep(Expr parser, MethodAccess ma) {
+  (
+    parser.getType().(RefType).getASourceSupertype*() instanceof TypeJwtParser or
+    parser.getType().(RefType).getASourceSupertype*() instanceof TypeJwtParserBuilder
+  ) and
+  ma.getQualifier() = parser
+}
+
+/**
+ * Holds if `parseHandlerExpr` is an insecure `JwtHandler`.
+ * That is, it overrides a method from `JwtHandlerOnJwtMethod` and
+ * the override is not defined on `JwtHandlerAdapter`.
+ * `JwtHandlerAdapter`'s overrides are safe since they always throw an exception.
+ */
+private predicate isInsecureJwtHandler(Expr parseHandlerExpr) {
+  exists(RefType t |
+    parseHandlerExpr.getType() = t and
+    t.getASourceSupertype*() instanceof TypeJwtHandler and
+    exists(Method m |
+      m = t.getAMethod() and
+      m.getASourceOverriddenMethod+() instanceof JwtHandlerOnJwtMethod and
+      not m.getSourceDeclaration() instanceof JwtHandlerAdapterOnJwtMethod
+    )
+  )
+}
+
+/** The interface `io.jsonwebtoken.JwtParser`. */
+private class TypeJwtParser extends Interface {
+  TypeJwtParser() { this.hasQualifiedName("io.jsonwebtoken", "JwtParser") }
+}
+
+/** The interface `io.jsonwebtoken.JwtParserBuilder`. */
+private class TypeJwtParserBuilder extends Interface {
+  TypeJwtParserBuilder() { this.hasQualifiedName("io.jsonwebtoken", "JwtParserBuilder") }
+}
+
+/** The interface `io.jsonwebtoken.JwtHandler`. */
+private class TypeJwtHandler extends Interface {
+  TypeJwtHandler() { this.hasQualifiedName("io.jsonwebtoken", "JwtHandler") }
+}
+
+/** The class `io.jsonwebtoken.JwtHandlerAdapter`. */
+private class TypeJwtHandlerAdapter extends Class {
+  TypeJwtHandlerAdapter() { this.hasQualifiedName("io.jsonwebtoken", "JwtHandlerAdapter") }
+}
+
+/** The `on(Claims|Plaintext)Jwt` methods defined in `JwtHandler`. */
+private class JwtHandlerOnJwtMethod extends Method {
+  JwtHandlerOnJwtMethod() {
+    this.hasName(["onClaimsJwt", "onPlaintextJwt"]) and
+    this.getNumberOfParameters() = 1 and
+    this.getDeclaringType() instanceof TypeJwtHandler
+  }
+}
+
+/** The `on(Claims|Plaintext)Jwt` methods defined in `JwtHandlerAdapter`. */
+private class JwtHandlerAdapterOnJwtMethod extends Method {
+  JwtHandlerAdapterOnJwtMethod() {
+    this.hasName(["onClaimsJwt", "onPlaintextJwt"]) and
+    this.getNumberOfParameters() = 1 and
+    this.getDeclaringType() instanceof TypeJwtHandlerAdapter
+  }
+}

java/ql/src/semmle/code/java/security/MissingJWTSignatureCheckQuery.qll

@@ -0,0 +1,23 @@
+/** Provides classes to be used in queries related to JSON Web Token (JWT) signature vulnerabilities. */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.security.JWT
+
+/**
+ * Models flow from signing keys assignments to qualifiers of JWT insecure parsers.
+ * This is used to determine whether a `JwtParser` performing unsafe parsing has a signing key set.
+ */
+class MissingJwtSignatureCheckConf extends DataFlow::Configuration {
+  MissingJwtSignatureCheckConf() { this = "SigningToExprDataFlow" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source instanceof JwtParserWithInsecureParseSource
+  }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof JwtParserWithInsecureParseSink }
+
+  override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(JwtParserWithInsecureParseAdditionalFlowStep c).step(node1, node2)
+  }
+}