Merge pull request github#12666 from smiddy007/improve-insufficient-pw-hash-query

JS: Improve insufficient pw hash query

Author: asgerf

javascript/ql/lib/change-notes/2023-03-26-CryptoJS-progressive-hashing.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The crypto-js module in `CryptoLibraries.qll` now supports progressive hashing with algo.update().

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

@@ -4,6 +4,7 @@
 
 import javascript
 import semmle.javascript.Concepts::Cryptography
+private import semmle.javascript.security.internal.CryptoAlgorithmNames
 
 /**
  * A key used in a cryptographic algorithm.
@@ -274,6 +275,47 @@ private module NodeJSCrypto {
  * A model of the crypto-js library.
  */
 private module CryptoJS {
+  private class InstantiatedAlgorithm extends DataFlow::CallNode {
+    private string algorithmName;
+
+    InstantiatedAlgorithm() {
+      /*
+       *       ```
+       *       const crypto = require("crypto-js");
+       *       const cipher = crypto.algo.SHA256.create();
+       *       ```
+       *       matched as:
+       *       ```
+       *       const crypto = require("crypto-js");
+       *       const cipher = crypto.algo.<algorithmName>.create();
+       *       ```
+       */
+
+      exists(DataFlow::SourceNode mod, DataFlow::PropRead propRead |
+        mod = DataFlow::moduleImport("crypto-js") and
+        propRead = mod.getAPropertyRead("algo").getAPropertyRead() and
+        this = propRead.getAMemberCall("create") and
+        algorithmName = propRead.getPropertyName() and
+        not isStrongPasswordHashingAlgorithm(algorithmName)
+      )
+    }
+
+    CryptographicAlgorithm getAlgorithm() { result.matchesName(algorithmName) }
+
+    private BlockMode getExplicitBlockMode() { result.matchesString(algorithmName) }
+
+    BlockMode getBlockMode() {
+      isBlockEncryptionAlgorithm(this.getAlgorithm()) and
+      (
+        if exists(this.getExplicitBlockMode())
+        then result = this.getExplicitBlockMode()
+        else
+          // CBC is the default if not explicitly specified
+          result = "CBC"
+      )
+    }
+  }
+
   /**
    *  Matches `CryptoJS.<algorithmName>` and `require("crypto-js/<algorithmName>")`
    */
@@ -325,13 +367,44 @@ private module CryptoJS {
     input = result.getParameter(0)
   }
 
+  private API::CallNode getUpdatedApplication(API::Node input, InstantiatedAlgorithm instantiation) {
+    /*
+     *    ```
+     *    var CryptoJS = require("crypto-js");
+     *    var hash = CryptoJS.algo.SHA256.create();
+     *    hash.update('message');
+     *    hash.update('password');
+     *    var hashInHex = hash.finalize();
+     *    ```
+     *    Matched as:
+     *    ```
+     *    var CryptoJS = require("crypto-js");
+     *    var hash = CryptoJS.algo.<algorithmName>.create();
+     *    hash.update(<input>);
+     *    hash.update(<input>);
+     *    var hashInHex = hash.finalize();
+     *    ```
+     *    Also matches where `CryptoJS.algo.<algorithmName>` has been
+     *    replaced by `require("crypto-js/<algorithmName>")`
+     */
+
+    result = instantiation.getAMemberCall("update") and
+    input = result.getParameter(0)
+  }
+
   private class Apply extends CryptographicOperation::Range instanceof API::CallNode {
     API::Node input;
     CryptographicAlgorithm algorithm; // non-functional
 
     Apply() {
-      this = getEncryptionApplication(input, algorithm) or
+      this = getEncryptionApplication(input, algorithm)
+      or
       this = getDirectApplication(input, algorithm)
+      or
+      exists(InstantiatedAlgorithm instantiation |
+        this = getUpdatedApplication(input, instantiation) and
+        algorithm = instantiation.getAlgorithm()
+      )
     }
 
     override DataFlow::Node getAnInput() { result = input.asSink() }

javascript/ql/src/Security/CWE-916/examples/InsufficientPasswordHash_CryptoJS.js

@@ -0,0 +1,8 @@
+const crypto = require('crypto-js')
+function hashPassword(email, password) {
+  var algo = crypto.algo.SHA512.create()
+  algo.update(password, 'utf-8') // BAD
+  algo.update(email.toLowerCase(), 'utf-8')
+  var hash = algo.finalize()
+  return hash.toString(crypto.enc.Base64)
+}

javascript/ql/src/Security/CWE-916/examples/InsufficientPasswordHash_CryptoJS_fixed

@@ -0,0 +1,8 @@
+const crypto = require('crypto-js')
+function hashPassword(email, password) {
+  var algo = crypto.algo.PBKDF2.create()
+  algo.update(password, 'utf-8') // GOOD
+  algo.update(email.toLowerCase(), 'utf-8')
+  var hash = algo.finalize()
+  return hash.toString(crypto.enc.Base64)
+}