Better comments and descriptions

Author: artem-smotrakov

java/ql/src/experimental/Security/CWE/CWE-208/NotConstantTimeCryptoComparison.qhelp

@@ -21,12 +21,12 @@ and does not depend on the contents of the arrays.
 <example>
 <p>
 The following example uses <code>Arrays.equals()</code> method for comparing MAC.
-This method implements a not-constant time algorithm:
+This method implements a non-constant time algorithm:
 </p>
 <sample src="UnsafeMacComparison.java" />
 
 <p>
-The next example example uses a safe not-constant time algorithm for comparing MAC:
+The next example uses a safe constant time algorithm for comparing MAC:
 </p>
 <sample src="SafeMacComparison.java" />
 
@@ -35,19 +35,11 @@ The next example example uses a safe not-constant time algorithm for comparing M
 <references>
 <li>
   Wikipedia:
-  <a href="https://en.wikipedia.org/wiki/Timing_attack">Timint attack</a>.
-</li>
-li>
-  Common Weakness Enumeration:
-  <a href="https://cwe.mitre.org/data/definitions/208.html">CWE-208: Observable Timing Discrepancy</a>.
-</li>
-<li>
-  Common Weakness Enumeration:
-  <a href="https://cwe.mitre.org/data/definitions/385.html">CWE-385: Covert Timing Channel</a>.
+  <a href="https://en.wikipedia.org/wiki/Timing_attack">Timing attack</a>.
 </li>
 <li>
   Java API Specification:
-  <a href="https://docs.oracle.com/javase/8/docs/api/java/security/MessageDigest.html#isEqual-byte:A-byte:A-">MessageDigest.isEqual() method</a>
+  <a href="https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/security/MessageDigest.html#isEqual(byte[],byte[])">MessageDigest.isEqual() method</a>
 </li>
 </references>
 </qhelp>

java/ql/src/experimental/Security/CWE/CWE-208/NotConstantTimeCryptoComparison.ql

@@ -1,13 +1,14 @@
 /**
- * @name Using a not-constant time algorithm for comparison results of a cryptographic operation
+ * @name Using a non-constant time algorithm for comparing results of a cryptographic operation
  * @description When comparing results of a cryptographic operation, a constant time algorithm should be used.
  *              Otherwise, an attacker may be able to implement a timing attack.
  *              A successful attack may result in leaking secrets or authentication bypass.
  * @kind path-problem
  * @problem.severity error
  * @precision high
- * @id java/not-constant-time-crypto-comparison
+ * @id java/non-constant-time-crypto-comparison
  * @tags security
+ *       external/cwe/cwe-385
  *       external/cwe/cwe-208
  */
 
@@ -16,11 +17,11 @@ import semmle.code.java.dataflow.TaintTracking
 import DataFlow::PathGraph
 
 /**
- * A method that returns a result of a cryptographic operation
+ * A method that returns the result of a cryptographic operation
  * such as encryption, decryption, signing, etc.
  */
-private class ReturnCryptoOperatinoResultMethod extends Method {
-  ReturnCryptoOperatinoResultMethod() {
+private class ReturnCryptoOperationResultMethod extends Method {
+  ReturnCryptoOperationResultMethod() {
     getDeclaringType().hasQualifiedName("javax.crypto", ["Mac", "Cipher"]) and
     hasName("doFinal")
     or
@@ -30,14 +31,14 @@ private class ReturnCryptoOperatinoResultMethod extends Method {
 }
 
 /**
- * A configuration that tracks data flows from cryptographic operations
- * to methods that compare data using a not-constant time algorithm.
+ * A configuration that tracks data flow from cryptographic operations
+ * to methods that compare data using a non-constant time algorithm.
  */
-private class NotConstantTimeCryptoComparisonConfig extends TaintTracking::Configuration {
-  NotConstantTimeCryptoComparisonConfig() { this = "NotConstantTimeCryptoComparisonConfig" }
+private class NonConstantTimeCryptoComparisonConfig extends TaintTracking::Configuration {
+  NonConstantTimeCryptoComparisonConfig() { this = "NonConstantTimeCryptoComparisonConfig" }
 
   override predicate isSource(DataFlow::Node source) {
-    exists(MethodAccess ma | ma.getMethod() instanceof ReturnCryptoOperatinoResultMethod |
+    exists(MethodAccess ma | ma.getMethod() instanceof ReturnCryptoOperationResultMethod |
       ma = source.asExpr()
     )
   }
@@ -59,7 +60,7 @@ private class NotConstantTimeCryptoComparisonConfig extends TaintTracking::Confi
   }
 }
 
-from DataFlow::PathNode source, DataFlow::PathNode sink, NotConstantTimeCryptoComparisonConfig conf
+from DataFlow::PathNode source, DataFlow::PathNode sink, NonConstantTimeCryptoComparisonConfig conf
 where conf.hasFlowPath(source, sink)
 select sink.getNode(), source, sink,
-  "Using a not-constant time algorithm for comparison results of a cryptographic operation."
+  "Using a non-constant time algorithm for comparing results of a cryptographic operation."