Merge branch 'main' into ts53-ts

2025-12-22 03:36:30 +01:00 · 2023-11-20 20:31:00 +01:00
parent 01e7d57dba 75f860595a
commit dde9a7cd7e
2268 changed files with 94487 additions and 30493 deletions
--- a/javascript/ql/lib/CHANGELOG.md
+++ b/javascript/ql/lib/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.8.3
+
+No user-facing changes.
+
 ## 0.8.2

 No user-facing changes.
--- a/javascript/ql/lib/change-notes/released/0.8.3.md
+++ b/javascript/ql/lib/change-notes/released/0.8.3.md
@@ -0,0 +1,3 @@
+## 0.8.3
+
+No user-facing changes.
--- a/javascript/ql/lib/codeql-pack.release.yml
+++ b/javascript/ql/lib/codeql-pack.release.yml
@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.8.2
+lastReleaseVersion: 0.8.3
--- a/javascript/ql/lib/qlpack.yml
+++ b/javascript/ql/lib/qlpack.yml
@@ -1,5 +1,5 @@
 name: codeql/javascript-all
-version: 0.8.3-dev
+version: 0.8.4-dev
 groups: javascript
 dbscheme: semmlecode.javascript.dbscheme
 extractor: javascript
--- a/javascript/ql/lib/semmle/javascript/frameworks/CryptoLibraries.qll
+++ b/javascript/ql/lib/semmle/javascript/frameworks/CryptoLibraries.qll
@@ -51,6 +51,7 @@ private module AsmCrypto {
  private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
    DataFlow::Node input;
    CryptographicAlgorithm algorithm; // non-functional
+    DataFlow::PropRead algorithmSelection;
    private string algorithmName;
    private string methodName;

@@ -68,11 +69,14 @@ private module AsmCrypto {
      exists(DataFlow::SourceNode asmCrypto |
        asmCrypto = DataFlow::globalVarRef("asmCrypto") and
        algorithm.matchesName(algorithmName) and
-        this = asmCrypto.getAPropertyRead(algorithmName).getAMemberCall(methodName) and
+        algorithmSelection = asmCrypto.getAPropertyRead(algorithmName) and
+        this = algorithmSelection.getAMemberCall(methodName) and
        input = this.getArgument(0)
      )
    }

+    override DataFlow::Node getInitialization() { result = algorithmSelection }
+
    override DataFlow::Node getAnInput() { result = input }

    override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -103,6 +107,7 @@ private module BrowserIdCrypto {

  private class Apply extends CryptographicOperation::Range instanceof DataFlow::MethodCallNode {
    CryptographicAlgorithm algorithm; // non-functional
+    DataFlow::CallNode keygen;

    Apply() {
      /*
@@ -122,8 +127,7 @@ private module BrowserIdCrypto {
       */

      exists(
-        DataFlow::SourceNode mod, DataFlow::Node algorithmNameNode, DataFlow::CallNode keygen,
-        DataFlow::FunctionNode callback
+        DataFlow::SourceNode mod, DataFlow::Node algorithmNameNode, DataFlow::FunctionNode callback
      |
        mod = DataFlow::moduleImport("browserid-crypto") and
        keygen = mod.getAMemberCall("generateKeypair") and
@@ -134,6 +138,8 @@ private module BrowserIdCrypto {
      )
    }

+    override DataFlow::Node getInitialization() { result = keygen }
+
    override DataFlow::Node getAnInput() { result = super.getArgument(0) }

    override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -239,6 +245,8 @@ private module NodeJSCrypto {

    Apply() { this = instantiation.getAMethodCall(any(string m | m = "update" or m = "write")) }

+    override DataFlow::Node getInitialization() { result = instantiation }
+
    override DataFlow::Node getAnInput() { result = super.getArgument(0) }

    override CryptographicAlgorithm getAlgorithm() { result = instantiation.getAlgorithm() }
@@ -324,7 +332,9 @@ private module CryptoJS {
    )
  }

-  private API::CallNode getEncryptionApplication(API::Node input, CryptographicAlgorithm algorithm) {
+  private API::CallNode getEncryptionApplication(
+    API::Node input, API::Node algorithmNode, CryptographicAlgorithm algorithm
+  ) {
    /*
     *    ```
     *    var CryptoJS = require("crypto-js");
@@ -338,11 +348,14 @@ private module CryptoJS {
     *    Also matches where `CryptoJS.<algorithmName>` has been replaced by `require("crypto-js/<algorithmName>")`
     */

-    result = getAlgorithmNode(algorithm).getMember("encrypt").getACall() and
+    algorithmNode = getAlgorithmNode(algorithm) and
+    result = algorithmNode.getMember("encrypt").getACall() and
    input = result.getParameter(0)
  }

-  private API::CallNode getDirectApplication(API::Node input, CryptographicAlgorithm algorithm) {
+  private API::CallNode getDirectApplication(
+    API::Node input, API::Node algorithmNode, CryptographicAlgorithm algorithm
+  ) {
    /*
     *    ```
     *    var CryptoJS = require("crypto-js");
@@ -357,7 +370,8 @@ private module CryptoJS {
     *    Also matches where `CryptoJS.<algorithmName>` has been replaced by `require("crypto-js/<algorithmName>")`
     */

-    result = getAlgorithmNode(algorithm).getACall() and
+    algorithmNode = getAlgorithmNode(algorithm) and
+    result = algorithmNode.getACall() and
    input = result.getParameter(0)
  }

@@ -389,18 +403,23 @@ private module CryptoJS {
  private class Apply extends CryptographicOperation::Range instanceof API::CallNode {
    API::Node input;
    CryptographicAlgorithm algorithm; // non-functional
+    DataFlow::Node instantiation;

    Apply() {
-      this = getEncryptionApplication(input, algorithm)
-      or
-      this = getDirectApplication(input, algorithm)
-      or
-      exists(InstantiatedAlgorithm instantiation |
-        this = getUpdatedApplication(input, instantiation) and
-        algorithm = instantiation.getAlgorithm()
+      exists(API::Node algorithmNode |
+        this = getEncryptionApplication(input, algorithmNode, algorithm)
+        or
+        this = getDirectApplication(input, algorithmNode, algorithm)
+      |
+        instantiation = algorithmNode.asSource()
      )
+      or
+      this = getUpdatedApplication(input, instantiation) and
+      algorithm = instantiation.(InstantiatedAlgorithm).getAlgorithm()
    }

+    override DataFlow::Node getInitialization() { result = instantiation }
+
    override DataFlow::Node getAnInput() { result = input.asSink() }

    override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -504,6 +523,8 @@ private module TweetNaCl {
      )
    }

+    override DataFlow::Node getInitialization() { result = this }
+
    override DataFlow::Node getAnInput() { result = input }

    override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -539,6 +560,7 @@ private module HashJs {
  private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
    DataFlow::Node input;
    CryptographicAlgorithm algorithm; // non-functional
+    DataFlow::CallNode init;

    Apply() {
      /*
@@ -554,10 +576,13 @@ private module HashJs {
       *      Also matches where `hash.<algorithmName>()` has been replaced by a more specific require a la `require("hash.js/lib/hash/sha/512")`
       */

-      this = getAlgorithmNode(algorithm).getAMemberCall("update") and
+      init = getAlgorithmNode(algorithm) and
+      this = init.getAMemberCall("update") and
      input = super.getArgument(0)
    }

+    override DataFlow::Node getInitialization() { result = init }
+
    override DataFlow::Node getAnInput() { result = input }

    override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -653,6 +678,8 @@ private module Forge {
      algorithm = cipher.getAlgorithm()
    }

+    override DataFlow::Node getInitialization() { result = cipher }
+
    override DataFlow::Node getAnInput() { result = input }

    override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -715,6 +742,8 @@ private module Md5 {
      super.getArgument(0) = input
    }

+    override DataFlow::Node getInitialization() { result = this }
+
    override DataFlow::Node getAnInput() { result = input }

    override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -731,17 +760,18 @@ private module Bcrypt {
  private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
    DataFlow::Node input;
    CryptographicAlgorithm algorithm;
+    API::Node init;

    Apply() {
      // `require("bcrypt").hash(password);` with minor naming variations
      algorithm.matchesName("BCRYPT") and
-      this =
-        API::moduleImport(["bcrypt", "bcryptjs", "bcrypt-nodejs"])
-            .getMember(["hash", "hashSync"])
-            .getACall() and
+      init = API::moduleImport(["bcrypt", "bcryptjs", "bcrypt-nodejs"]) and
+      this = init.getMember(["hash", "hashSync"]).getACall() and
      super.getArgument(0) = input
    }

+    override DataFlow::Node getInitialization() { result = init.asSource() }
+
    override DataFlow::Node getAnInput() { result = input }

    override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -769,6 +799,8 @@ private module Hasha {
      )
    }

+    override DataFlow::Node getInitialization() { result = this }
+
    override DataFlow::Node getAnInput() { result = input }

    override CryptographicAlgorithm getAlgorithm() { result = algorithm }
--- a/javascript/ql/lib/semmle/javascript/internal/ConceptsShared.qll
+++ b/javascript/ql/lib/semmle/javascript/internal/ConceptsShared.qll
@@ -40,6 +40,9 @@ module Cryptography {
    /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
    CryptographicAlgorithm getAlgorithm() { result = super.getAlgorithm() }

+    /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
+    DataFlow::Node getInitialization() { result = super.getInitialization() }
+
    /** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */
    DataFlow::Node getAnInput() { result = super.getAnInput() }

@@ -65,6 +68,9 @@ module Cryptography {
     * extend `CryptographicOperation` instead.
     */
    abstract class Range extends DataFlow::Node {
+      /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
+      abstract DataFlow::Node getInitialization();
+
      /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
      abstract CryptographicAlgorithm getAlgorithm();

--- a/javascript/ql/lib/semmle/javascript/security/dataflow/BrokenCryptoAlgorithmCustomizations.qll
+++ b/javascript/ql/lib/semmle/javascript/security/dataflow/BrokenCryptoAlgorithmCustomizations.qll
@@ -19,7 +19,10 @@ module BrokenCryptoAlgorithm {
  /**
   * A data flow sink for sensitive information in broken or weak cryptographic algorithms.
   */
-  abstract class Sink extends DataFlow::Node { }
+  abstract class Sink extends DataFlow::Node {
+    /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
+    abstract DataFlow::Node getInitialization();
+  }

  /**
   * A sanitizer for sensitive information in broken or weak cryptographic algorithms.
@@ -38,15 +41,17 @@ module BrokenCryptoAlgorithm {
   * An expression used by a broken or weak cryptographic algorithm.
   */
  class WeakCryptographicOperationSink extends Sink {
+    CryptographicOperation application;
+
    WeakCryptographicOperationSink() {
-      exists(CryptographicOperation application |
-        (
-          application.getAlgorithm().isWeak()
-          or
-          application.getBlockMode().isWeak()
-        ) and
-        this = application.getAnInput()
-      )
+      (
+        application.getAlgorithm().isWeak()
+        or
+        application.getBlockMode().isWeak()
+      ) and
+      this = application.getAnInput()
    }
+
+    override DataFlow::Node getInitialization() { result = application.getInitialization() }
  }
 }
--- a/javascript/ql/lib/semmle/javascript/security/regexp/ExponentialBackTracking.qll
+++ b/javascript/ql/lib/semmle/javascript/security/regexp/ExponentialBackTracking.qll
@@ -3,7 +3,7 @@
 *
 *   James Kirrage, Asiri Rathnayake, Hayo Thielecke: Static Analysis for
 *     Regular Expression Denial-of-Service Attacks. NSS 2013.
- *     (http://www.cs.bham.ac.uk/~hxt/research/reg-exp-sec.pdf)
+ *     (https://arxiv.org/abs/1301.0849)
 *   Asiri Rathnayake, Hayo Thielecke: Static Analysis for Regular Expression
 *     Exponential Runtime via Substructural Logics. 2014.
 *     (https://www.cs.bham.ac.uk/~hxt/research/redos_full.pdf)
--- a/javascript/ql/src/CHANGELOG.md
+++ b/javascript/ql/src/CHANGELOG.md
@@ -1,3 +1,10 @@
+## 0.8.3
+
+### Query Metadata Changes
+
+* Lower the severity of log-injection to medium.
+* Increase the severity of XSS to high.
+
 ## 0.8.2

 ### Minor Analysis Improvements
--- a/javascript/ql/src/Performance/ReDoSReferences.inc.qhelp
+++ b/javascript/ql/src/Performance/ReDoSReferences.inc.qhelp
@@ -10,7 +10,7 @@
 		<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/ReDoS">ReDoS</a>.</li>
 		<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/Time_complexity">Time complexity</a>.</li>
 		<li>James Kirrage, Asiri Rathnayake, Hayo Thielecke:
-		<a href="http://www.cs.bham.ac.uk/~hxt/research/reg-exp-sec.pdf">Static Analysis for Regular Expression Denial-of-Service Attack</a>.
+		<a href="https://arxiv.org/abs/1301.0849">Static Analysis for Regular Expression Denial-of-Service Attack</a>.
 		</li>
 	</references>
 </qhelp>
--- a/javascript/ql/src/Security/CWE-079/ReflectedXss.ql
+++ b/javascript/ql/src/Security/CWE-079/ReflectedXss.ql
@@ -4,7 +4,7 @@
 *              a cross-site scripting vulnerability.
 * @kind path-problem
 * @problem.severity error
- * @security-severity 6.1
+ * @security-severity 7.8
 * @precision high
 * @id js/reflected-xss
 * @tags security
--- a/javascript/ql/src/Security/CWE-079/StoredXss.ql
+++ b/javascript/ql/src/Security/CWE-079/StoredXss.ql
@@ -4,7 +4,7 @@
 *              a stored cross-site scripting vulnerability.
 * @kind path-problem
 * @problem.severity error
- * @security-severity 6.1
+ * @security-severity 7.8
 * @precision high
 * @id js/stored-xss
 * @tags security
--- a/javascript/ql/src/Security/CWE-079/Xss.ql
+++ b/javascript/ql/src/Security/CWE-079/Xss.ql
@@ -4,7 +4,7 @@
 *              a cross-site scripting vulnerability.
 * @kind path-problem
 * @problem.severity error
- * @security-severity 6.1
+ * @security-severity 7.8
 * @precision high
 * @id js/xss
 * @tags security
--- a/javascript/ql/src/Security/CWE-117/LogInjection.ql
+++ b/javascript/ql/src/Security/CWE-117/LogInjection.ql
@@ -4,7 +4,7 @@
 *              insertion of forged log entries by a malicious user.
 * @kind path-problem
 * @problem.severity error
- * @security-severity 7.8
+ * @security-severity 6.1
 * @precision medium
 * @id js/log-injection
 * @tags security
--- a/javascript/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql
+++ b/javascript/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql
@@ -16,9 +16,14 @@ import semmle.javascript.security.dataflow.BrokenCryptoAlgorithmQuery
 import semmle.javascript.security.SensitiveActions
 import DataFlow::PathGraph

-from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+from
+  Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Source sourceNode,
+  Sink sinkNode
 where
  cfg.hasFlowPath(source, sink) and
-  not source.getNode() instanceof CleartextPasswordExpr // flagged by js/insufficient-password-hash
-select sink.getNode(), source, sink, "A broken or weak cryptographic algorithm depends on $@.",
-  source.getNode(), "sensitive data from " + source.getNode().(Source).describe()
+  sourceNode = source.getNode() and
+  sinkNode = sink.getNode() and
+  not sourceNode instanceof CleartextPasswordExpr // flagged by js/insufficient-password-hash
+select sinkNode, source, sink, "$@ depends on $@.", sinkNode.getInitialization(),
+  "A broken or weak cryptographic algorithm", sourceNode,
+  "sensitive data from " + sourceNode.describe()
--- a/javascript/ql/src/Summary/LinesOfCode.ql
+++ b/javascript/ql/src/Summary/LinesOfCode.ql
@@ -4,6 +4,7 @@
 * @description The total number of lines of JavaScript or TypeScript code across all files checked into the repository, except in `node_modules`. This is a useful metric of the size of a database. For all files that were seen during extraction, this query counts the lines of code, excluding whitespace or comments.
 * @kind metric
 * @tags summary
+ *       telemetry
 */

 import javascript
--- a/javascript/ql/src/change-notes/released/0.8.3.md
+++ b/javascript/ql/src/change-notes/released/0.8.3.md
@@ -0,0 +1,6 @@
+## 0.8.3
+
+### Query Metadata Changes
+
+* Lower the severity of log-injection to medium.
+* Increase the severity of XSS to high.
--- a/javascript/ql/src/codeql-pack.release.yml
+++ b/javascript/ql/src/codeql-pack.release.yml
@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.8.2
+lastReleaseVersion: 0.8.3
--- a/javascript/ql/src/qlpack.yml
+++ b/javascript/ql/src/qlpack.yml
@@ -1,5 +1,5 @@
 name: codeql/javascript-queries
-version: 0.8.3-dev
+version: 0.8.4-dev
 groups:
  - javascript
  - queries
--- a/javascript/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected
@@ -26,8 +26,8 @@ edges
 | tst.js:19:17:19:24 | password | tst.js:19:17:19:24 | password |
 | tst.js:22:21:22:30 | secretText | tst.js:22:21:22:30 | secretText |
 #select
-| tst.js:11:17:11:26 | secretText | tst.js:3:18:3:24 | trusted | tst.js:11:17:11:26 | secretText | A broken or weak cryptographic algorithm depends on $@. | tst.js:3:18:3:24 | trusted | sensitive data from an access to trusted |
-| tst.js:11:17:11:26 | secretText | tst.js:11:17:11:26 | secretText | tst.js:11:17:11:26 | secretText | A broken or weak cryptographic algorithm depends on $@. | tst.js:11:17:11:26 | secretText | sensitive data from an access to secretText |
-| tst.js:17:17:17:25 | o.trusted | tst.js:17:17:17:25 | o.trusted | tst.js:17:17:17:25 | o.trusted | A broken or weak cryptographic algorithm depends on $@. | tst.js:17:17:17:25 | o.trusted | sensitive data from an access to trusted |
-| tst.js:22:21:22:30 | secretText | tst.js:3:18:3:24 | trusted | tst.js:22:21:22:30 | secretText | A broken or weak cryptographic algorithm depends on $@. | tst.js:3:18:3:24 | trusted | sensitive data from an access to trusted |
-| tst.js:22:21:22:30 | secretText | tst.js:22:21:22:30 | secretText | tst.js:22:21:22:30 | secretText | A broken or weak cryptographic algorithm depends on $@. | tst.js:22:21:22:30 | secretText | sensitive data from an access to secretText |
+| tst.js:11:17:11:26 | secretText | tst.js:3:18:3:24 | trusted | tst.js:11:17:11:26 | secretText | $@ depends on $@. | tst.js:5:19:5:49 | crypto. ... ', key) | A broken or weak cryptographic algorithm | tst.js:3:18:3:24 | trusted | sensitive data from an access to trusted |
+| tst.js:11:17:11:26 | secretText | tst.js:11:17:11:26 | secretText | tst.js:11:17:11:26 | secretText | $@ depends on $@. | tst.js:5:19:5:49 | crypto. ... ', key) | A broken or weak cryptographic algorithm | tst.js:11:17:11:26 | secretText | sensitive data from an access to secretText |
+| tst.js:17:17:17:25 | o.trusted | tst.js:17:17:17:25 | o.trusted | tst.js:17:17:17:25 | o.trusted | $@ depends on $@. | tst.js:5:19:5:49 | crypto. ... ', key) | A broken or weak cryptographic algorithm | tst.js:17:17:17:25 | o.trusted | sensitive data from an access to trusted |
+| tst.js:22:21:22:30 | secretText | tst.js:3:18:3:24 | trusted | tst.js:22:21:22:30 | secretText | $@ depends on $@. | tst.js:21:22:21:60 | crypto. ... ', key) | A broken or weak cryptographic algorithm | tst.js:3:18:3:24 | trusted | sensitive data from an access to trusted |
+| tst.js:22:21:22:30 | secretText | tst.js:22:21:22:30 | secretText | tst.js:22:21:22:30 | secretText | $@ depends on $@. | tst.js:21:22:21:60 | crypto. ... ', key) | A broken or weak cryptographic algorithm | tst.js:22:21:22:30 | secretText | sensitive data from an access to secretText |