implement a simple InsufficientKeySize query

Author: erik-krogh

javascript/ql/lib/semmle/javascript/frameworks/CryptoLibraries.qll

@@ -25,6 +25,26 @@ abstract class CryptographicOperation extends Expr {
  */
 abstract class CryptographicKey extends DataFlow::ValueNode { }
 
+/**
+ * The creation of a cryptographic key.
+ */
+abstract class CryptographicKeyCreation extends DataFlow::Node {
+  /**
+   * Gets the algorithm used to create the key.
+   */
+  abstract CryptographicAlgorithm getAlgorithm();
+
+  /**
+   * Gets the size of the key.
+   */
+  abstract int getSize();
+
+  /**
+   * Gets whether the key is symmetric.
+   */
+  abstract predicate isSymmetricKey();
+}
+
 /**
  * A key used in a cryptographic algorithm, viewed as a `CredentialsExpr`.
  */
@@ -151,6 +171,39 @@ private module NodeJSCrypto {
     CryptographicAlgorithm getAlgorithm() { result = algorithm }
   }
 
+  private class CreateKey extends CryptographicKeyCreation, DataFlow::CallNode {
+    boolean symmetric;
+
+    CreateKey() {
+      // crypto.generateKey(type, options, callback)
+      // crypto.generateKeyPair(type, options, callback)
+      // crypto.generateKeyPairSync(type, options)
+      // crypto.generateKeySync(type, options)
+      exists(DataFlow::SourceNode mod, string keyType |
+        keyType = "Key" and symmetric = true
+        or
+        keyType = "KeyPair" and symmetric = false
+      |
+        mod = DataFlow::moduleImport("crypto") and
+        this = mod.getAMemberCall("generate" + keyType + ["", "Sync"])
+      )
+    }
+
+    override CryptographicAlgorithm getAlgorithm() {
+      result.matchesName(getArgument(0).getStringValue())
+    }
+
+    override int getSize() {
+      symmetric = true and
+      result = getOptionArgument(1, "length").getIntValue()
+      or
+      symmetric = false and
+      result = getOptionArgument(1, "modulusLength").getIntValue()
+    }
+
+    override predicate isSymmetricKey() { symmetric = true }
+  }
+
   private class Apply extends CryptographicOperation, MethodCallExpr {
     InstantiatedAlgorithm instantiation;
 

javascript/ql/src/Security/CWE-326/InsufficientKeySize.ql

@@ -0,0 +1,36 @@
+/**
+ * @name Use of a weak cryptographic key
+ * @description Using a weak cryptographic key can allow an attacker to compromise security.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 7.5
+ * @precision high
+ * @id js/insufficient-key-size
+ * @tags security
+ *       external/cwe/cwe-326
+ */
+
+import javascript
+
+from CryptographicKeyCreation key, int size, string msg, string algo
+where
+  size = key.getSize() and
+  (
+    algo = key.getAlgorithm() + " "
+    or
+    not exists(key.getAlgorithm()) and algo = ""
+  ) and
+  (
+    size < 128 and
+    key.isSymmetricKey() and
+    msg =
+      "Creation of an symmetric " + algo + "key uses " + size +
+        " bits, which is below 128 and considered breakable."
+    or
+    size < 2048 and
+    not key.isSymmetricKey() and
+    msg =
+      "Creation of an asymmetric " + algo + "key uses " + size +
+        " bits, which is below 2048 and considered breakable."
+  )
+select key, msg

javascript/ql/test/query-tests/Security/CWE-326/InsufficientKeySize.expected

@@ -0,0 +1,2 @@
+| tst.js:3:14:3:71 | crypto. ... 1024 }) | Creation of an asymmetric RSA key uses 1024 bits, which is below 2048 and considered breakable. |
+| tst.js:7:14:7:59 | crypto. ... : 64 }) | Creation of an symmetric key uses 64 bits, which is below 128 and considered breakable. |

javascript/ql/test/query-tests/Security/CWE-326/InsufficientKeySize.qlref

@@ -0,0 +1 @@
+Security/CWE-326/InsufficientKeySize.ql

javascript/ql/test/query-tests/Security/CWE-326/tst.js

@@ -0,0 +1,9 @@
+const crypto = require("crypto");
+
+const bad1 = crypto.generateKeyPairSync("rsa", { modulusLength: 1024 }); // NOT OK
+
+const good1 = crypto.generateKeyPairSync("rsa", { modulusLength: 4096 }); // OK
+
+const bad2 = crypto.generateKeySync("hmac", { length: 64 }); // NOT OK
+
+const good2 = crypto.generateKeySync("aes", { length: 256 }); // OK