add insufficient key size model for node-forge

Author: erik-krogh

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

@@ -544,6 +544,39 @@ private module Forge {
   private class Key extends CryptographicKey {
     Key() { this = any(KeyCipher cipher).getKey() }
   }
+
+  private class CreateKey extends CryptographicKeyCreation, DataFlow::CallNode {
+    CryptographicAlgorithm algorithm;
+
+    CreateKey() {
+      // var cipher = forge.rc2.createEncryptionCipher(key, 128);
+      this =
+        getAnImportNode()
+            .getAPropertyRead(any(string s | algorithm.matchesName(s)))
+            .getAMemberCall("createEncryptionCipher")
+      or
+      // var key = forge.random.getBytesSync(16);
+      // var cipher = forge.cipher.createCipher('AES-CBC', key);
+      this =
+        getAnImportNode()
+            .getAPropertyRead("cipher")
+            .getAMemberCall(["createCipher", "createDecipher"]) and
+      algorithm.matchesName(this.getArgument(0).getStringValue())
+    }
+
+    override CryptographicAlgorithm getAlgorithm() { result = algorithm }
+
+    override int getSize() {
+      result = this.getArgument(1).getIntValue()
+      or
+      exists(DataFlow::CallNode call | call.getCalleeName() = ["getBytes", "getBytesSync"] |
+        getArgument(1).getALocalSource() = call and
+        result = call.getArgument(0).getIntValue() * 8 // bytes to bits
+      )
+    }
+
+    override predicate isSymmetricKey() { any() }
+  }
 }
 
 /**

javascript/ql/lib/semmle/javascript/security/CryptoAlgorithms.qll

@@ -31,14 +31,22 @@ private module AlgorithmNames {
   }
 
   predicate isStrongEncryptionAlgorithm(string name) {
-    name = ["AES", "AES128", "AES192", "AES256", "AES512", "RSA", "RABBIT", "BLOWFISH"]
+    name = [appendMode("AES"), "AES128", "AES192", "AES256", "AES512", "RSA", "RABBIT", "BLOWFISH"]
+  }
+
+  /**
+   * Gets the name with a mode of operation added as a suffix.
+   */
+  bindingset[name]
+  private string appendMode(string name) {
+    result = name + ["", "CBC", "ECB", "CFB", "OFB", "CTR", "GCM"]
   }
 
   predicate isWeakEncryptionAlgorithm(string name) {
     name =
       [
-        "DES", "3DES", "TRIPLEDES", "TDEA", "TRIPLEDEA", "ARC2", "RC2", "ARC4", "RC4", "ARCFOUR",
-        "ARC5", "RC5"
+        appendMode("DES"), appendMode("3DES"), "TRIPLEDES", "TDEA", "TRIPLEDEA", "ARC2", "RC2",
+        "ARC4", "RC4", "ARCFOUR", "ARC5", "RC5"
       ]
   }
 

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

@@ -1,5 +1,8 @@
 | 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. |
-| tst.js:14:14:14:56 | CryptoJ ... e: 2 }) | Creation of an symmetric PBKDF2 key uses 64 bits, which is below 128 and considered breakable. |
-| tst.js:15:14:15:60 | CryptoJ ... e: 2 }) | Creation of an symmetric PBKDF2 key uses 64 bits, which is below 128 and considered breakable. |
-| tst.js:16:14:16:60 | CryptoJ ... e: 2 }) | Creation of an symmetric EVPKDF key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:13:14:13:56 | CryptoJ ... e: 2 }) | Creation of an symmetric PBKDF2 key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:14:14:14:60 | CryptoJ ... e: 2 }) | Creation of an symmetric PBKDF2 key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:15:14:15:60 | CryptoJ ... e: 2 }) | Creation of an symmetric EVPKDF key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:19:12:19:57 | forge.r ... rd, 64) | Creation of an symmetric RC2 key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:26:12:26:53 | forge.c ... , key2) | Creation of an symmetric AESCBC key uses 64 bits, which is below 128 and considered breakable. |
+| tst.js:30:12:30:56 | forge.c ... , key3) | Creation of an symmetric 3DESCBC key uses 64 bits, which is below 128 and considered breakable. |

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

@@ -13,4 +13,21 @@ var CryptoJS = require("crypto-js");
 const bad3 = CryptoJS.algo.PBKDF2.create({ keySize: 2 }); // NOT OK
 const bad4 = CryptoJS.PBKDF2(password, salt, { keySize: 2 }); // NOT OK
 const bad5 = CryptoJS.EvpKDF(password, salt, { keySize: 2 }); // NOT OK
-const bad4 = CryptoJS.PBKDF2(password, salt, { keySize: 8 }); // OK
+const bad6 = CryptoJS.PBKDF2(password, salt, { keySize: 8 }); // OK
+
+const forge = require("node-forge");
+var bad7 = forge.rc2.createEncryptionCipher(password, 64); // NOT OK
+var good3 = forge.rc2.createEncryptionCipher(password, 128); // OK
+
+var key1 = forge.random.getBytesSync(16);
+var good4 = forge.cipher.createCipher('AES-CBC', key1); // OK
+
+var key2 = forge.random.getBytesSync(8);
+var bad8 = forge.cipher.createCipher('AES-CBC', key2); // NOT OK
+
+var myBuffer = forge.util.createBuffer(manyBytes);
+var key3 = myBuffer.getBytes(8);
+var bad9 = forge.cipher.createDecipher('3DES-CBC', key3); // NOT OK
+
+var key4 = myBuffer.getBytes(16);
+var good5 = forge.cipher.createDecipher('AES-CBC', key4); // OK