Apply suggestions from code review

Co-authored-by: Chris Smowton <[email protected]>

Author: intrigus-lgtm

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

@@ -2,7 +2,7 @@
 <qhelp>
 
 <overview>
-<p> A JWT consists of three parts: header, payload, and signature.
+<p> A JSON Web Token (JWT) consists of three parts: header, payload, and signature.
 The <code>io.jsonwebtoken.jjwt</code> library is one of many libraries used for working with JWTs.
 It offers different methods for parsing tokens like <code>parse</code>, <code>parseClaimsJws</code>, and <code>parsePlaintextJws</code>.
 The last two correctly verify that the JWT is properly signed.
@@ -12,7 +12,7 @@ comparing the locally computed signature with the signature part of the JWT.
 <p>
 Therefore it is necessary to provide the <code>JwtParser</code> with a key that is used for signature validation.
 Unfortunately the <code>parse</code> method <b>accepts</b> a JWT whose signature is empty although a signing key has been set for the parser.
-This means that an attacker can create arbitrary JWTs that will be accepted.
+This means that an attacker can create arbitrary JWTs that will be accepted if this method is used.
 </p>
 </overview>
 <recommendation>
@@ -36,4 +36,4 @@ The third and fourth good cases use <code>parseClaimsJws</code> method or overri
 <references>
 <li>zofrex: <a href="https://www.zofrex.com/blog/2020/10/20/alg-none-jwt-nhs-contact-tracing-app/">How I Found An alg=none JWT Vulnerability in the NHS Contact Tracing App</a>.</li>
 </references>
-</qhelp>
+</qhelp>

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

@@ -70,10 +70,10 @@ private class JwtHandlerAdapterOnJwtMethod extends Method {
 
 /**
  * Holds if `parseHandlerExpr` is an insecure `JwtHandler`.
- * That is, it overrides a method from `JwtHandlerOnJwtMethod` and the overridden method is not a method from `JwtHandlerAdapterOnJwtMethod`.
- * A overridden method which is a method from `JwtHandlerAdapterOnJwtMethod` is safe, because these always throw an exception.
+ * 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 isInsecureParseHandler(Expr parseHandlerExpr) {
+private predicate isInsecureJwtHandler(Expr parseHandlerExpr) {
   exists(RefType t |
     parseHandlerExpr.getType() = t and
     t.getASourceSupertype*() instanceof TypeJwtHandler and
@@ -95,7 +95,7 @@ private class JwtParserInsecureParseMethodAccess extends MethodAccess {
     this.getMethod().getASourceOverriddenMethod*() instanceof JwtParserInsecureParseMethod
     or
     this.getMethod().getASourceOverriddenMethod*() instanceof JwtParserParseHandlerMethod and
-    isInsecureParseHandler(this.getArgument(1))
+    isInsecureJwtHandler(this.getArgument(1))
   }
 }
 
@@ -113,7 +113,7 @@ private class JwtParserInsecureParseMethodAccess extends MethodAccess {
  * ```
  * In this case, the signing key is set on a `JwtParserBuilder` indirectly setting the key of `JwtParser` that is created by the call to `build`.
  */
-private predicate isSigningKeySet(Expr expr, MethodAccess signingMa) {
+private predicate isSigningKeySetter(Expr expr, MethodAccess signingMa) {
   any(SigningToExprDataFlow s).hasFlow(DataFlow::exprNode(signingMa), DataFlow::exprNode(expr))
 }
 
@@ -123,7 +123,7 @@ private class JwtParserWithSigningKeyExpr extends Expr {
 
   JwtParserWithSigningKeyExpr() {
     this.getType().(RefType).getASourceSupertype*() instanceof TypeJwtParser and
-    isSigningKeySet(this, signingMa)
+    isSigningKeySetter(this, signingMa)
   }
 
   /** Gets the method access that sets the signing key for this parser. */