Merge branch 'main' into amammad-js-bombs

javascript/ql/src/CHANGELOG.md

@@ -1,3 +1,90 @@
+## 0.8.6
+
+No user-facing changes.
+
+## 0.8.5
+
+No user-facing changes.
+
+## 0.8.4
+
+### Minor Analysis Improvements
+
+* Added django URLs to detected "safe" URL patterns in `js/unsafe-external-link`. 
+
+## 0.8.3
+
+### Query Metadata Changes
+
+* Lower the security severity of log-injection to medium.
+* Increase the security severity of XSS to high.
+
+## 0.8.2
+
+### Minor Analysis Improvements
+
+* Added modeling for importing `express-rate-limit` using a named import.
+
+## 0.8.1
+
+### Minor Analysis Improvements
+
+* Added the `AmdModuleDefinition::Range` class, making it possible to define custom aliases for the AMD `define` function.
+
+## 0.8.0
+
+No user-facing changes.
+
+## 0.7.5
+
+### Bug Fixes
+
+* Fixed an extractor crash that could occur in projects containing TypeScript files larger than 10 MB.
+
+## 0.7.4
+
+### Minor Analysis Improvements
+
+* Files larger than 10 MB are no longer be extracted or analyzed.
+* Imports can now be resolved in more cases, where a non-constant string expression is passed to a `require()` call.
+
+### Bug Fixes
+
+* Fixed an extractor crash that would occur in rare cases when a TypeScript file contains a self-referential namespace alias.
+
+## 0.7.3
+
+No user-facing changes.
+
+## 0.7.2
+
+No user-facing changes.
+
+## 0.7.1
+
+### Minor Analysis Improvements
+
+* The `fs/promises` package is now recognised as an alias for `require('fs').promises`.
+* The `js/path-injection` query can now track taint through calls to `path.join()` with a spread argument, such as `path.join(baseDir, ...args)`.
+
+## 0.7.0
+
+### Bug Fixes
+
+* The query "Arbitrary file write during zip extraction ("Zip Slip")" (`js/zipslip`) has been renamed to "Arbitrary file access during archive extraction ("Zip Slip")."
+
+## 0.6.4
+
+No user-facing changes.
+
+## 0.6.3
+
+### Minor Analysis Improvements
+
+* Fixed an issue where calls to a method named `search` would lead to false positive alerts related to regular expressions.
+  This happened when the call was incorrectly seen as a call to `String.prototype.search`, since this function converts its first argument
+  to a regular expression. The analysis is now more restrictive about when to treat `search` calls as regular expression sinks.
+
 ## 0.6.2
 
 ### Major Analysis Improvements

javascript/ql/src/DOM/TargetBlank.ql

@@ -44,7 +44,9 @@ predicate hasDynamicHrefHostAttributeValue(DOM::ElementDefinition elem) {
       // ... that does not start with a fixed host or a relative path (common formats)
       not url.regexpMatch("(?i)((https?:)?//)?[-a-z0-9.]*/.*") and
       // .. that is not a call to `url_for` in a Flask / nunjucks application
-      not url.regexpMatch("\\{\\{\\s*url(_for)?\\(.+\\).*")
+      not url.regexpMatch("\\{\\{\\s*url(_for)?\\(.+\\).*") and
+      // .. that is not a call to `url` in a Django application
+      not url.regexpMatch("\\{%\\s*url.*")
     )
   )
 }

javascript/ql/src/Declarations/AssignmentToConst.ql

@@ -13,8 +13,9 @@
 import javascript
 import semmle.javascript.RestrictedLocations
 
-from ConstDeclStmt cds, VariableDeclarator decl, VarDef def, Variable v
+from DeclStmt cds, VariableDeclarator decl, VarDef def, Variable v
 where
+  (cds instanceof ConstDeclStmt or cds instanceof UsingDeclStmt) and
   decl = cds.getADecl() and
   def.getAVariable() = v and
   decl.getBindingPattern().getAVariable() = v and

javascript/ql/src/Diagnostics/SuccessfullyExtractedFiles.ql

@@ -1,6 +1,6 @@
 /**
- * @name Successfully extracted files
- * @description Lists all files in the source code directory that were extracted without encountering an error in the file.
+ * @name Extracted files
+ * @description Lists all files in the source code directory that were extracted.
  * @kind diagnostic
  * @id js/diagnostics/successfully-extracted-files
  * @tags successfully-extracted-files
@@ -9,7 +9,5 @@
 import javascript
 
 from File f
-where
-  not exists(Error e | e.isFatal() and e.getFile() = f) and
-  exists(f.getRelativePath())
+where exists(f.getRelativePath())
 select f, ""

javascript/ql/src/Expressions/TypoDatabase.qll

@@ -1,4 +0,0 @@
-import codeql.typos.TypoDatabase as DB
-
-/** DEPRECATED: Use the `codeql/typos` pack instead. */
-deprecated predicate typos = DB::typos/2;

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>

javascript/ql/src/RegExp/IdentityReplacement.ql

@@ -3,7 +3,7 @@
  * @description Replacing a substring with itself has no effect and may indicate a mistake.
  * @kind problem
  * @problem.severity warning
- * @security-severity 7.8
+ * @security-severity 5.0
  * @id js/identity-replacement
  * @precision very-high
  * @tags correctness

javascript/ql/src/Security/CWE-022/TaintedPath.qhelp

@@ -13,40 +13,47 @@ attacker being able to influence behavior by modifying unexpected files.
 
 <recommendation>
 <p>
-Validate user input before using it to construct a file path, either using an off-the-shelf library
-like the <code>sanitize-filename</code> npm package, or by performing custom validation.
+Validate user input before using it to construct a file path.
 </p>
 
 <p>
-Ideally, follow these rules:
+The validation method you should use depends on whether you want to allow the user to specify complex paths with multiple components that may span multiple folders, or only simple filenames without a path component.
 </p>
 
-<ul>
-<li>Do not allow more than a single "." character.</li>
-<li>Do not allow directory separators such as "/" or "\" (depending on the file system).</li>
-<li>Do not rely on simply replacing problematic sequences such as "../". For example, after
-applying this filter to ".../...//", the resulting string would still be "../".</li>
-<li>Use a whitelist of known good patterns.</li>
-</ul>
+<p>
+In the former case, a common strategy is to make sure that the constructed file path is contained within a safe root folder.
+First, normalize the path using <code>path.resolve</code> or <code>fs.realpathSync</code> to remove any ".." segments.
+You should always normalize the file path since an unnormalized path that starts with the root folder can still be used to access files outside the root folder.
+Then, after you have normalized the path, check that the path starts with the root folder.
+</p>
+
+<p>
+In the latter case, you can use a library like the <code>sanitize-filename</code> npm package to eliminate any special characters from the file path.
+Note that it is <i>not</i> sufficient to only remove "../" sequences: for example, applying this filter to ".../...//" would still result in the string "../".
+</p>
+
+<p>
+Finally, the simplest (but most restrictive) option is to use an allow list of safe patterns and make sure that the user input matches one of these patterns.
+</p>
 </recommendation>
 
 <example>
 <p>
-In the first example, a file name is read from an HTTP request and then used to access a file.
-However, a malicious user could enter a file name which is an absolute path, such as
-<code>"/etc/passwd"</code>.
-</p>
-
-<p>
-In the second example, it appears that the user is restricted to opening a file within the
-<code>"user"</code> home directory. However, a malicious user could enter a file name containing
-special characters. For example, the string <code>"../../etc/passwd"</code> will result in the code
-reading the file located at <code>"/home/user/../../etc/passwd"</code>, which is the system's
-password file. This file would then be sent back to the user, giving them access to all the
-system's passwords.
+In the first (bad) example, the code reads the file name from an HTTP request, then accesses that file within a root folder.
+A malicious user could enter a file name containing "../" segments to navigate outside the root folder and access sensitive files.
 </p>
 
 <sample src="examples/TaintedPath.js" />
+
+<p>
+The second (good) example shows how to avoid access to sensitive files by sanitizing the file path.
+First, the code resolves the file name relative to a root folder, normalizing the path and removing any "../" segments in the process.
+Then, the code calls <code>fs.realpathSync</code> to resolve any symbolic links in the path.
+Finally, the code checks that the normalized path starts with the path of the root folder, ensuring the file is contained within the root folder.
+</p>
+
+<sample src="examples/TaintedPathGood.js" />
+
 </example>
 
 <references>

javascript/ql/src/Security/CWE-022/examples/TaintedPath.js

@@ -1,13 +1,12 @@
-var fs = require('fs'),
-    http = require('http'),
-    url = require('url');
+const fs = require('fs'),
+      http = require('http'),
+      url = require('url');
+
+const ROOT = "/var/www/";
 
 var server = http.createServer(function(req, res) {
-  let path = url.parse(req.url, true).query.path;
+  let filePath = url.parse(req.url, true).query.path;
 
-  // BAD: This could read any file on the file system
-  res.write(fs.readFileSync(path));
-
-  // BAD: This could still read any file on the file system
-  res.write(fs.readFileSync("/home/user/" + path));
-});
+  // BAD: This function uses unsanitized input that can read any file on the file system.
+  res.write(fs.readFileSync(ROOT + filePath, 'utf8'));
+});

javascript/ql/src/Security/CWE-022/examples/TaintedPathGood.js

@@ -0,0 +1,19 @@
+const fs = require('fs'),
+      http = require('http'),
+      path = require('path'),
+      url = require('url');
+
+const ROOT = "/var/www/";
+
+var server = http.createServer(function(req, res) {
+  let filePath = url.parse(req.url, true).query.path;
+
+  // GOOD: Verify that the file path is under the root directory
+  filePath = fs.realpathSync(path.resolve(ROOT, filePath));
+  if (!filePath.startsWith(ROOT)) {
+    res.statusCode = 403;
+    res.end();
+    return;
+  }
+  res.write(fs.readFileSync(filePath, 'utf8'));
+});

javascript/ql/src/Security/CWE-078/CommandInjection.inc.qhelp

@@ -2,43 +2,65 @@
   "-//Semmle//qhelp//EN"
   "qhelp.dtd">
 <qhelp>
+
 <overview>
-<p>Code that passes user input directly to
-<code>require('child_process').exec</code>, or some other library
-routine that executes a command, allows the user to execute malicious
-code.</p>
-
+<p>Code that passes untrusted user input directly to
+<code>child_process.exec</code> or similar APIs that execute shell commands
+allows the user to execute malicious code.</p>
 </overview>
+
 <recommendation>
+<p>If possible, use APIs that don't run shell commands and that accept command
+arguments as an array of strings rather than a single concatenated string. This
+is both safer and more portable.</p>
 
-<p>If possible, use hard-coded string literals to specify the command to run
-or library to load. Instead of passing the user input directly to the
-process or library function, examine the user input and then choose
-among hard-coded string literals.</p>
-
-<p>If the applicable libraries or commands cannot be determined at
-compile time, then add code to verify that the user input string is
-safe before using it.</p>
+<p>If given arguments as a single string, avoid simply splitting the string on 
+whitespace. Arguments may contain quoted whitespace, causing them to split into 
+multiple arguments. Use a library like <code>shell-quote</code> to parse the string 
+into an array of arguments instead.</p>
 
+<p>If this approach is not viable, then add code to verify that the user input
+string is safe before using it.</p>
 </recommendation>
-<example>
 
-<p>The following example shows code that takes a shell script that can be changed
-maliciously by a user, and passes it straight to <code>child_process.exec</code>
-without examining it first.</p>
+<example>
+<p>The following example shows code that extracts a filename from an HTTP query
+parameter that may contain untrusted data, and then embeds it into a shell
+command to count its lines without examining it first:</p>
 
 <sample src="examples/command-injection.js" />
 
-</example>
-<references>
+<p>A malicious user can take advantage of this code by executing arbitrary shell commands. For example, by providing a filename like <code>foo.txt; rm -rf .</code>, the user can first count the lines in <code>foo.txt</code> and subsequently delete all files in the current directory. </p>
 
+<p>To avoid this catastrophic behavior, use an API such as
+<code>child_process.execFileSync</code> that does not spawn a shell by
+default:</p>
+
+<sample src="examples/command-injection_fixed.js" />
+
+<p>If you want to allow the user to specify other options to <code>wc</code>,
+you can use a library like <code>shell-quote</code> to parse the user input into
+an array of arguments without risking command injection:</p>
+
+<sample src="examples/command-injection_shellquote.js" />
+
+<p>Alternatively, the original example can be made safe by checking the filename
+against an allowlist of safe characters before using it:</p>
+
+<sample src="examples/command-injection_allowlist.js" />
+</example>
+
+<references>
 <li>
 OWASP:
 <a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
 </li>
-
+<li>
+npm:
+<a href="https://www.npmjs.com/package/shell-quote">shell-quote</a>.
+</li>
 <!--  LocalWords:  CWE untrusted unsanitized Runtime
  -->
-
 </references>
+
 </qhelp>

javascript/ql/src/Security/CWE-078/IndirectCommandInjection.qhelp

@@ -27,27 +27,25 @@
 
 		<p>
 
-			If possible, use hard-coded string literals to specify the
-			command to run or library to load. Instead of forwarding the
-			command-line arguments to the process, examine the command-line
-			arguments and then choose among hard-coded string literals.
+			If possible, use APIs that don't run shell commands and accept
+			command arguments as an array of strings rather than a single
+			concatenated string. This is both safer and more portable.
 
 		</p>
 
 		<p>
 
-			If the applicable libraries or commands cannot be determined
-			at compile time, then add code to verify that each forwarded
-			command-line argument is properly escaped before using it.
+If given arguments as a single string, avoid simply splitting the string on 
+whitespace. Arguments may contain quoted whitespace, causing them to split into 
+multiple arguments. Use a library like <code>shell-quote</code> to parse the string 
+into an array of arguments instead.
 
 		</p>
 
 		<p>
 
-			If the forwarded command-line arguments are part of the
-			arguments of the system command, prefer a library routine that handles
-			the arguments as an array of strings rather than a single concatenated
-			string. This prevents the unexpected evaluation of special characters.
+			If this approach is not viable, then add code to verify that each
+			forwarded command-line argument is properly escaped before using it.
 
 		</p>
 
@@ -91,6 +89,17 @@
 
 		<sample src="examples/indirect-command-injection_fixed.js" />
 
+		<p>
+
+			If you want to allow the user to specify other options to
+			<code>node</code>, you can use a library like
+			<code>shell-quote</code> to parse the user input into an array of
+			arguments without risking command injection:
+
+		</p>
+
+		<sample src="examples/indirect-command-injection_shellquote.js" />
+
 	</example>
 
 	<references>
@@ -100,6 +109,11 @@
 			<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
 		</li>
 
+		<li>
+			npm:
+			<a href="https://www.npmjs.com/package/shell-quote">shell-quote</a>.
+		</li>
+
 	</references>
 
 </qhelp>

javascript/ql/src/Security/CWE-078/UnsafeShellCommandConstruction.qhelp

@@ -27,9 +27,22 @@
 	</p>
 
 	<p>
-		Alternatively, if the shell command must be constructed
-		dynamically, then add code to ensure that special characters 
-		do not alter the shell command unexpectedly.
+
+		If given arguments as a single string, avoid simply splitting the string
+		on whitespace. Arguments may contain quoted whitespace, causing them to
+		split into multiple arguments. Use a library like
+		<code>shell-quote</code> to parse the string into an array of arguments
+		instead.
+
+	</p>
+
+	<p>
+
+		Alternatively, if the command must be interpreted by a shell (for
+		example because it includes I/O redirections), you can use
+		<code>shell-quote</code> to escape any special characters in the input
+		before embedding it in the command.
+
 	</p>
 
 </recommendation>
@@ -63,6 +76,27 @@
 
 	<sample src="examples/unsafe-shell-command-construction_fixed.js" />
 
+	<p>
+
+		As another example, consider the following code which is similar to the
+		preceding example, but pipes the output of <code>wget</code> into <code>wc -l</code>
+		to count the number of lines in the downloaded file.
+
+	</p>
+
+	<sample src="examples/unsafe-shell-command-construction_pipe.js" />
+
+	<p>
+
+		In this case, using <code>child_process.execFile</code> is not an option
+		because the shell is needed to interpret the pipe operator. Instead, you
+		can use <code>shell-quote</code> to escape the input before embedding it
+		in the command:
+
+	</p>
+
+	<sample src="examples/unsafe-shell-command-construction_pipe_fixed.js" />
+
 </example>
 <references>
 
@@ -71,5 +105,10 @@
 		<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
 	</li>
 
+	<li>
+		npm:
+		<a href="https://www.npmjs.com/package/shell-quote">shell-quote</a>.
+	</li>
+
 </references>
 </qhelp>

javascript/ql/src/Security/CWE-078/examples/command-injection.js

@@ -3,7 +3,7 @@ var cp = require("child_process"),
     url = require('url');
 
 var server = http.createServer(function(req, res) {
-    let cmd = url.parse(req.url, true).query.path;
+    let file = url.parse(req.url, true).query.path;
 
-    cp.exec(cmd); // BAD
+    cp.execSync(`wc -l ${file}`); // BAD
 });

javascript/ql/src/Security/CWE-078/examples/command-injection_allowlist.js

@@ -0,0 +1,12 @@
+var cp = require("child_process"),
+    http = require('http'),
+    url = require('url');
+
+var server = http.createServer(function(req, res) {
+    let file = url.parse(req.url, true).query.path;
+
+    // only allow safe characters in file name
+    if (file.match(/^[\w\.\-\/]+$/)) {
+        cp.execSync(`wc -l ${file}`); // GOOD
+    }
+});

javascript/ql/src/Security/CWE-078/examples/command-injection_fixed.js

@@ -0,0 +1,9 @@
+var cp = require("child_process"),
+    http = require('http'),
+    url = require('url');
+
+var server = http.createServer(function(req, res) {
+    let file = url.parse(req.url, true).query.path;
+
+    cp.execFileSync('wc', ['-l', file]); // GOOD
+});

javascript/ql/src/Security/CWE-078/examples/command-injection_shellquote.js

@@ -0,0 +1,10 @@
+var cp = require("child_process"),
+    http = require('http'),
+    url = require('url'),
+    shellQuote = require('shell-quote');
+
+var server = http.createServer(function(req, res) {
+    let options = url.parse(req.url, true).query.options;
+
+    cp.execFileSync('wc', shellQuote.parse(options)); // GOOD
+});

javascript/ql/src/Security/CWE-078/examples/indirect-command-injection.js

@@ -2,4 +2,4 @@ var cp = require("child_process");
 
 const args = process.argv.slice(2);
 const script = path.join(__dirname, 'bin', 'main.js');
-cp.execSync(`node ${script} ${args.join(' ')}"`); // BAD
+cp.execSync(`node ${script} ${args.join(' ')}`); // BAD

javascript/ql/src/Security/CWE-078/examples/indirect-command-injection_shellquote.js

@@ -0,0 +1,11 @@
+var cp = require("child_process"),
+    shellQuote = require("shell-quote");
+
+const args = process.argv.slice(2);
+let nodeOpts = '';
+if (args[0] === '--node-opts') {
+    nodeOpts = args[1];
+    args.splice(0, 2);
+}
+const script = path.join(__dirname, 'bin', 'main.js');
+cp.execFileSync('node', shellQuote.parse(nodeOpts).concat(script).concat(args)); // GOOD

javascript/ql/src/Security/CWE-078/examples/unsafe-shell-command-construction.js

@@ -1,5 +1,5 @@
 var cp = require("child_process");
 
 module.exports = function download(path, callback) {
-  cp.execSync("wget " + path, callback);
+  cp.exec("wget " + path, callback);
 }

javascript/ql/src/Security/CWE-078/examples/unsafe-shell-command-construction_fixed.js

@@ -1,5 +1,5 @@
 var cp = require("child_process");
 
 module.exports = function download(path, callback) {
-  cp.execFileSync("wget", [path], callback);
+  cp.execFile("wget", [path], callback);
 }

javascript/ql/src/Security/CWE-078/examples/unsafe-shell-command-construction_pipe.js

@@ -0,0 +1,5 @@
+var cp = require("child_process");
+
+module.exports = function download(path, callback) {
+  cp.exec("wget " + path + " | wc -l", callback);
+};

javascript/ql/src/Security/CWE-078/examples/unsafe-shell-command-construction_pipe_fixed.js

@@ -0,0 +1,5 @@
+var cp = require("child_process");
+
+module.exports = function download(path, callback) {
+  cp.exec("wget " + shellQuote.quote([path]) + " | wc -l", callback);
+};

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

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

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

javascript/ql/src/Security/CWE-094/examples/comment_issue_good.yml

@@ -7,4 +7,4 @@ jobs:
     - env:
         BODY: ${{ github.event.issue.body }}
       run: |
-        echo '$BODY'
+        echo "$BODY"

javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.qhelp

@@ -1,8 +1,137 @@
 <!DOCTYPE qhelp PUBLIC
-"-//Semmle//qhelp//EN"
-"qhelp.dtd">
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
 <qhelp>
 
-	<include src="IncompleteSanitization.qhelp" />
+<overview>
+<p>
+Sanitizing untrusted input is a common technique for preventing injection attacks and other security 
+vulnerabilities. Regular expressions are often used to perform this sanitization. However, when the 
+regular expression matches multiple consecutive characters, replacing it just once 
+can result in the unsafe text reappearing in the sanitized input.
+</p>
+<p>
+Attackers can exploit this issue by crafting inputs that, when sanitized with an ineffective regular 
+expression, still contain malicious code or content. This can lead to code execution, data exposure, 
+or other vulnerabilities.
+</p>
+</overview>
 
+<recommendation>
+<p>
+To prevent this issue, it is highly recommended to use a well-tested sanitization library whenever 
+possible. These libraries are more likely to handle corner cases and ensure effective sanitization.
+</p>
+
+<p>
+If a library is not an option, you can consider alternative strategies to fix the issue. For example, 
+applying the regular expression replacement repeatedly until no more replacements can be performed, or rewriting the regular 
+expression to match single characters instead of the entire unsafe text.
+</p>
+</recommendation>
+
+<example>
+<p>
+Consider the following JavaScript code that aims to remove all HTML comment start and end tags:
+</p>
+
+<sample language="javascript">
+str.replace(/&lt;!--|--!?&gt;/g, "");   
+</sample>
+
+<p>
+Given the input string "&lt;!&lt;!--- comment ---&gt;&gt;", the output will be "&lt;!-- comment --&gt;", 
+which still contains an HTML comment.
+</p>
+
+<p>
+One possible fix for this issue is to apply the regular expression replacement repeatedly until no
+more replacements can be performed. This ensures that the unsafe text does not re-appear in the sanitized input, effectively 
+removing all instances of the targeted pattern:
+</p>
+
+<sample language="javascript">
+function removeHtmlComments(input) {  
+  let previous;  
+  do {  
+    previous = input;  
+    input = input.replace(/&lt;!--|--!?&gt;/g, "");  
+  } while (input !== previous);  
+  return input;  
+}  
+</sample>
+</example>
+
+<example>
+<p>
+Another example is the following regular expression intended to remove script tags:
+</p>
+
+<sample language="javascript">
+str.replace(/&lt;script\b[^&lt;]*(?:(?!&lt;\/script&gt;)&lt;[^&lt;]*)*&lt;\/script&gt;/g, "");  
+</sample>
+
+<p>
+If the input string is "&lt;scrip&lt;script&gt;is removed&lt;/script&gt;t&gt;alert(123)&lt;/script&gt;", 
+the output will be "&lt;script&gt;alert(123)&lt;/script&gt;", which still contains a script tag.
+</p>
+<p>
+A fix for this issue is to rewrite the regular expression to match single characters 
+("&lt;" and "&gt;") instead of the entire unsafe text. This simplifies the sanitization process 
+and ensures that all potentially unsafe characters are removed:
+</p>
+<sample language="javascript">
+function removeAllHtmlTags(input) {  
+  return input.replace(/&lt;|&gt;/g, "");  
+}
+</sample>
+<p>
+Another potential fix is to use the popular <code>sanitize-html</code> npm library. 
+It keeps most of the safe HTML tags while removing all unsafe tags and attributes.
+</p>
+<sample language="javascript">
+const sanitizeHtml = require("sanitize-html");
+function removeAllHtmlTags(input) {  
+  return sanitizeHtml(input);  
+}
+</sample>
+
+</example>
+
+<example>
+<p>
+Lastly, consider a path sanitizer using the regular expression <code>/\.\.\//</code>:
+</p>
+
+<sample language="javascript">
+str.replace(/\.\.\//g, "");  
+</sample>
+
+<p>
+The regular expression attempts to strip out all occurences of <code>/../</code> from <code>str</code>.
+This will not work as expected: for the string <code>/./.././</code>, for example, it will remove the single
+occurrence of <code>/../</code> in the middle, but the remainder of the string then becomes
+<code>/../</code>, which is another instance of the substring we were trying to remove.
+</p>
+
+<p>
+A possible fix for this issue is to use the "sanitize-filename" npm library for path sanitization. 
+This library is specifically designed to handle path sanitization, and should handle all corner cases
+and ensure effective sanitization:
+</p>
+
+<sample language="javascript">
+const sanitize = require("sanitize-filename");  
+  
+function sanitizePath(input) {  
+  return sanitize(input);  
+}  
+</sample>
+
+</example>
+
+<references>
+<li>OWASP Top 10: <a href="https://www.owasp.org/index.php/Top_10-2017_A1-Injection">A1 Injection</a>.</li>
+<li>Stack Overflow: <a href="https://stackoverflow.com/questions/6659351/removing-all-script-tags-from-html-with-js-regular-expression">Removing all script tags from HTML with JS regular expression</a>.</li>
+</references>
 </qhelp>

javascript/ql/src/Security/CWE-116/IncompleteSanitization.qhelp

@@ -43,18 +43,6 @@ needed, for instance by using prepared statements for SQL queries.
 Otherwise, make sure to use a regular expression with the <code>g</code> flag to ensure that
 all occurrences are replaced, and remember to escape backslashes if applicable.
 </p>
-<p>
-Note, however, that this is generally <i>not</i> sufficient for replacing multi-character strings:
-the <code>String.prototype.replace</code> method only performs one pass over the input string,
-and will not replace further instances of the string that result from earlier replacements.
-</p>
-<p>
-For example, consider the code snippet <code>s.replace(/\/\.\.\//g, "")</code>, which attempts
-to strip out all occurences of <code>/../</code> from <code>s</code>. This will not work as
-expected: for the string <code>/./.././</code>, for example, it will remove the single
-occurrence of <code>/../</code> in the middle, but the remainder of the string then becomes
-<code>/../</code>, which is another instance of the substring we were trying to remove.
-</p>
 </recommendation>
 
 <example>

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

javascript/ql/src/Security/CWE-327/BadRandomness.ql

@@ -1,5 +1,5 @@
 /**
- * @name Creating biased random numbers from a cryptographically secure source.
+ * @name Creating biased random numbers from a cryptographically secure source
  * @description Some mathematical operations on random numbers can cause bias in
  *              the results and compromise security.
  * @kind problem

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()

javascript/ql/src/Security/CWE-400/RemotePropertyInjection.expected

@@ -1 +0,0 @@
-| examples/RemotePropertyInjection.js:8:8:8:11 | prop | A $@ is used as a property name to write to. | examples/RemotePropertyInjection.js:7:13:7:36 | req.que ... trolled | user-provided value |

javascript/ql/src/Security/CWE-601/ServerSideUrlRedirect.qhelp

@@ -16,6 +16,10 @@ To guard against untrusted URL redirection, it is advisable to avoid putting use
 directly into a redirect URL. Instead, maintain a list of authorized
 redirects on the server; then choose from that list based on the user input provided.
 </p>
+<p>
+If this is not possible, then the user input should be validated in some other way,
+for example, by verifying that the target URL is on the same host as the current page.
+</p>
 </recommendation>
 
 <example>
@@ -32,6 +36,21 @@ before doing the redirection:
 </p>
 
 <sample src="examples/ServerSideUrlRedirectGood.js"/>
+
+<p>
+Alternatively, we can check that the target URL does not redirect to a different host
+by parsing it relative to a base URL with a known host and verifying that the host
+stays the same:
+</p>
+
+<sample src="examples/ServerSideUrlRedirectGood2.js"/>
+
+<p>
+Note that as written, the above code will allow redirects to URLs on <code>example.com</code>,
+which is harmless but perhaps not intended. You can substitute your own domain (if known) for
+<code>example.com</code> to prevent this.
+</p>
+
 </example>
 
 <references>

javascript/ql/src/Security/CWE-601/examples/ServerSideUrlRedirect.js

@@ -1,6 +1,6 @@
 const app = require("express")();
 
-app.get('/some/path', function(req, res) {
+app.get("/redirect", function (req, res) {
   // BAD: a request parameter is incorporated without validation into a URL redirect
-  res.redirect(req.param("target"));
+  res.redirect(req.query["target"]);
 });

javascript/ql/src/Security/CWE-601/examples/ServerSideUrlRedirectGood.js

@@ -2,9 +2,12 @@ const app = require("express")();
 
 const VALID_REDIRECT = "http://cwe.mitre.org/data/definitions/601.html";
 
-app.get('/some/path', function(req, res) {
+app.get("/redirect", function (req, res) {
   // GOOD: the request parameter is validated against a known fixed string
-  let target = req.param("target");
-  if (VALID_REDIRECT === target)
+  let target = req.query["target"];
+  if (VALID_REDIRECT === target) {
     res.redirect(target);
+  } else {
+    res.redirect("/");
+  }
 });

javascript/ql/src/Security/CWE-601/examples/ServerSideUrlRedirectGood2.js

@@ -0,0 +1,22 @@
+const app = require("express")();
+
+function isLocalUrl(path) {
+  try {
+    return (
+      // TODO: consider substituting your own domain for example.com
+      new URL(path, "https://example.com").origin === "https://example.com"
+    );
+  } catch (e) {
+    return false;
+  }
+}
+
+app.get("/redirect", function (req, res) {
+  // GOOD: check that we don't redirect to a different host
+  let target = req.query["target"];
+  if (isLocalUrl(target)) {
+    res.redirect(target);
+  } else {
+    res.redirect("/");
+  }
+});

javascript/ql/src/Security/CWE-730/ServerCrash.qhelp

@@ -42,15 +42,13 @@
 
 	<p>
 
-		The following server implementation checks if a client-provided
-		file path is valid and throws an exception if the check fails. It can
-		be seen that the exception is uncaught, and it is therefore reasonable to
-		expect the server to respond with an error response to client requests
-		that cause the check to fail.
-
-		But since the exception is uncaught in the context of an
-		asynchronous callback invocation (<code>fs.access(...)</code>), the
-		entire server will terminate instead.
+		The following server code checks if a client-provided file path is valid
+		before saving data to that path. It would be reasonable to expect that the
+		server responds with an error in case the request contains an invalid
+		file path. However, the server instead throws an exception, which is
+		uncaught in the context of the asynchronous callback invocation
+		(<code>fs.access(...)</code>). This causes the entire server to
+		terminate abruptly.
 
 	</p>
 
@@ -67,11 +65,9 @@
 
 	<p>
 
-		An alternative is to use an <code>async</code> and
-		<code>await</code> for the asynchronous behavior, since the server
-		will then print warning messages about uncaught exceptions instead of
-		terminating, unless the server was started with the commandline option
-		<code>--unhandled-rejections=strict</code>:
+		To simplify exception handling, it may be advisable to switch to
+		async/await syntax instead of using callbacks, which allows wrapping the
+		entire request handler in a <code>try/catch</code> block:
 
 	</p>
 

javascript/ql/src/Security/CWE-730/examples/server-crash.BAD.js

@@ -7,10 +7,13 @@ function save(rootDir, path, content) {
   }
   // write content to disk
 }
+
 express().post("/save", (req, res) => {
-  fs.exists(rootDir, (exists) => {
-    if (!exists) {
-      console.error(`Server setup is corrupted, ${rootDir} does not exist!`);
+  fs.access(rootDir, (err) => {
+    if (err) {
+      console.error(
+        `Server setup is corrupted, ${rootDir} cannot be accessed!`
+      );
       res.status(500);
       res.end();
       return;

javascript/ql/src/Security/CWE-730/examples/server-crash.GOOD-A.js

@@ -1,9 +1,9 @@
 // ...
 express().post("/save", (req, res) => {
-  fs.exists(rootDir, (exists) => {
+  fs.access(rootDir, (err) => {
     // ...
     try {
-      save(rootDir, req.query.path, req.body); // GOOD no uncaught exception
+      save(rootDir, req.query.path, req.body); // GOOD exception is caught below
       res.status(200);
       res.end();
     } catch (e) {

javascript/ql/src/Security/CWE-730/examples/server-crash.GOOD-B.js

@@ -1,12 +1,12 @@
 // ...
 express().post("/save", async (req, res) => {
-  if (!(await fs.promises.exists(rootDir))) {
-    console.error(`Server setup is corrupted, ${rootDir} does not exist!`);
+  try {
+    await fs.promises.access(rootDir);
+    save(rootDir, req.query.path, req.body); // GOOD exception is caught below
+    res.status(200);
+    res.end();
+  } catch (e) {
     res.status(500);
     res.end();
-    return;
   }
-  save(rootDir, req.query.path, req.body); // MAYBE BAD, depends on the commandline options
-  res.status(200);
-  res.end();
 });

javascript/ql/src/Security/CWE-770/examples/MissingRateLimitingGood.js

@@ -4,8 +4,8 @@ var app = express();
 // set up rate limiter: maximum of five requests per minute
 var RateLimit = require('express-rate-limit');
 var limiter = RateLimit({
-  windowMs: 1*60*1000, // 1 minute
-  max: 5
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  max: 100, // max 100 requests per windowMs
 });
 
 // apply rate limiter to all requests

javascript/ql/src/Security/CWE-798/HardcodedCredentials.qhelp

@@ -21,6 +21,23 @@
 </p>
 </recommendation>
 
+<example>
+  <p>
+    The following code example connects to an HTTP request using an hard-codes authentication header: 
+  </p>
+
+  <sample src="examples/HardcodedCredentialsHttpRequest.js"/>
+
+  <p>
+    Instead, user name and password can be supplied through the environment variables
+    <code>username</code> and <code>password</code>, which can be set externally without hard-coding
+    credentials in the source code.
+  </p>
+
+  <sample src="examples/HardcodedCredentialsHttpRequestFixed.js"/>
+
+</example>
+
 <example>
   <p>
     The following code example connects to a Postgres database using the <code>pg</code> package

javascript/ql/src/Security/CWE-798/examples/HardcodedCredentialsHttpRequest.js

@@ -0,0 +1,18 @@
+let base64 = require('base-64');
+
+let url = 'http://example.org/auth';
+let username = 'user';
+let password = 'passwd';
+
+let headers = new Headers();
+
+headers.append('Content-Type', 'text/json');
+headers.append('Authorization', 'Basic' + base64.encode(username + ":" + password));
+
+fetch(url, {
+          method:'GET',
+          headers: headers
+       })
+.then(response => response.json())
+.then(json => console.log(json))
+.done();

javascript/ql/src/Security/CWE-798/examples/HardcodedCredentialsHttpRequestFixed.js

@@ -0,0 +1,18 @@
+let base64 = require('base-64');
+
+let url = 'http://example.org/auth';
+let username = process.env.USERNAME;
+let password = process.env.PASSWORD;
+
+let headers = new Headers();
+
+headers.append('Content-Type', 'text/json');
+headers.append('Authorization', 'Basic' + base64.encode(username + ":" + password));
+
+fetch(url, {
+        method:'GET',
+        headers: headers
+     })
+.then(response => response.json())
+.then(json => console.log(json))
+.done();

javascript/ql/src/Statements/UselessConditional.ql

@@ -39,6 +39,8 @@ predicate isSymbolicConstant(Variable v) {
   exists(VarDef vd | vd = getSingleDef(v) |
     vd.(VariableDeclarator).getDeclStmt() instanceof ConstDeclStmt
     or
+    vd.(VariableDeclarator).getDeclStmt() instanceof UsingDeclStmt
+    or
     isConstant(vd.getSource())
   )
 }

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

javascript/ql/src/change-notes/2023-12-18-dot-templates.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added support for [doT](https://github.com/olado/doT) templates. 

javascript/ql/src/change-notes/released/0.6.3.md

@@ -1,6 +1,7 @@
----
-category: minorAnalysis
----
+## 0.6.3
+
+### Minor Analysis Improvements
+
 * Fixed an issue where calls to a method named `search` would lead to false positive alerts related to regular expressions.
   This happened when the call was incorrectly seen as a call to `String.prototype.search`, since this function converts its first argument
   to a regular expression. The analysis is now more restrictive about when to treat `search` calls as regular expression sinks.

javascript/ql/src/change-notes/released/0.6.4.md

@@ -0,0 +1,3 @@
+## 0.6.4
+
+No user-facing changes.

javascript/ql/src/change-notes/released/0.7.0.md

@@ -1,4 +1,5 @@
----
-category: fix
----
+## 0.7.0
+
+### Bug Fixes
+
 * The query "Arbitrary file write during zip extraction ("Zip Slip")" (`js/zipslip`) has been renamed to "Arbitrary file access during archive extraction ("Zip Slip")."

javascript/ql/src/change-notes/released/0.7.1.md

@@ -0,0 +1,6 @@
+## 0.7.1
+
+### Minor Analysis Improvements
+
+* The `fs/promises` package is now recognised as an alias for `require('fs').promises`.
+* The `js/path-injection` query can now track taint through calls to `path.join()` with a spread argument, such as `path.join(baseDir, ...args)`.

javascript/ql/src/change-notes/released/0.7.2.md

@@ -0,0 +1,3 @@
+## 0.7.2
+
+No user-facing changes.

javascript/ql/src/change-notes/released/0.7.3.md

@@ -0,0 +1,3 @@
+## 0.7.3
+
+No user-facing changes.

javascript/ql/src/change-notes/released/0.7.4.md

@@ -0,0 +1,10 @@
+## 0.7.4
+
+### Minor Analysis Improvements
+
+* Files larger than 10 MB are no longer be extracted or analyzed.
+* Imports can now be resolved in more cases, where a non-constant string expression is passed to a `require()` call.
+
+### Bug Fixes
+
+* Fixed an extractor crash that would occur in rare cases when a TypeScript file contains a self-referential namespace alias.

javascript/ql/src/change-notes/released/0.7.5.md

@@ -0,0 +1,5 @@
+## 0.7.5
+
+### Bug Fixes
+
+* Fixed an extractor crash that could occur in projects containing TypeScript files larger than 10 MB.

javascript/ql/src/change-notes/released/0.8.0.md

@@ -0,0 +1,3 @@
+## 0.8.0
+
+No user-facing changes.

javascript/ql/src/change-notes/released/0.8.1.md

@@ -0,0 +1,5 @@
+## 0.8.1
+
+### Minor Analysis Improvements
+
+* Added the `AmdModuleDefinition::Range` class, making it possible to define custom aliases for the AMD `define` function.

javascript/ql/src/change-notes/released/0.8.2.md

@@ -0,0 +1,5 @@
+## 0.8.2
+
+### Minor Analysis Improvements
+
+* Added modeling for importing `express-rate-limit` using a named import.

javascript/ql/src/change-notes/released/0.8.3.md

@@ -0,0 +1,6 @@
+## 0.8.3
+
+### Query Metadata Changes
+
+* Lower the security severity of log-injection to medium.
+* Increase the security severity of XSS to high.

javascript/ql/src/change-notes/released/0.8.4.md

@@ -0,0 +1,5 @@
+## 0.8.4
+
+### Minor Analysis Improvements
+
+* Added django URLs to detected "safe" URL patterns in `js/unsafe-external-link`. 

javascript/ql/src/change-notes/released/0.8.5.md

@@ -0,0 +1,3 @@
+## 0.8.5
+
+No user-facing changes.

javascript/ql/src/change-notes/released/0.8.6.md

@@ -0,0 +1,3 @@
+## 0.8.6
+
+No user-facing changes.

javascript/ql/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.6.2
+lastReleaseVersion: 0.8.6

javascript/ql/src/experimental/Security/CWE-918/SSRF.qll

@@ -29,8 +29,8 @@ class Configuration extends TaintTracking::Configuration {
     )
   }
 
-  override predicate isSanitizerEdge(DataFlow::Node source, DataFlow::Node sink) {
-    this.strictSanitizingPrefixEdge(source, sink)
+  override predicate isSanitizerOut(DataFlow::Node node) {
+    this.strictSanitizingPrefixEdge(node, _)
   }
 
   override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode nd) {

javascript/ql/src/experimental/semmle/javascript/Actions.qll

@@ -1,4 +0,0 @@
-/** DEPRECATED: Use `semmle.javascript.Actions` instead. */
-deprecated module Actions {
-  import semmle.javascript.Actions::Actions
-}

javascript/ql/src/experimental/semmle/javascript/SQL.qll

@@ -0,0 +1,157 @@
+/**
+ * Provides classes for working with SQL connectors.
+ */
+
+import javascript
+
+module ExperimentalSql {
+  /**
+   * Provides SQL injection Sinks for the [TypeORM](https://www.npmjs.com/package/typeorm) package
+   */
+  private module TypeOrm {
+    /**
+     * Gets a `DataSource` instance
+     *
+     * `DataSource` is a pre-defined connection configuration to a specific database.
+     */
+    API::Node dataSource() {
+      result = API::moduleImport("typeorm").getMember("DataSource").getInstance()
+    }
+
+    /**
+     * Gets a `QueryRunner` instance
+     */
+    API::Node queryRunner() { result = dataSource().getMember("createQueryRunner").getReturn() }
+
+    /**
+     * Gets a `*QueryBuilder` instance
+     */
+    API::Node queryBuilderInstance() {
+      // a `*QueryBuilder` instance of a Data Mapper based Entity
+      result =
+        [
+          // Using DataSource
+          dataSource(),
+          // Using repository
+          dataSource().getMember("getRepository").getReturn(),
+          // Using entity manager
+          dataSource().getMember("manager"), queryRunner().getMember("manager")
+        ].getMember("createQueryBuilder").getReturn()
+      or
+      // A `*QueryBuilder` instance of an Active record based Entity
+      result =
+        API::moduleImport("typeorm")
+            .getMember("Entity")
+            .getReturn()
+            .getADecoratedClass()
+            .getMember("createQueryBuilder")
+            .getReturn()
+      or
+      // A WhereExpressionBuilder can be used in complex WHERE expression
+      result =
+        API::moduleImport("typeorm")
+            .getMember(["Brackets", "NotBrackets"])
+            .getParameter(0)
+            .getParameter(0)
+      or
+      // In case of custom query builders
+      result =
+        API::moduleImport("typeorm")
+            .getMember([
+                "SelectQueryBuilder", "InsertQueryBuilder", "RelationQueryBuilder",
+                "UpdateQueryBuilder", "WhereExpressionBuilder"
+              ])
+            .getInstance()
+    }
+
+    /**
+     *  Gets function names which create any type of `QueryBuilder` like `WhereExpressionBuilder` or `InsertQueryBuilder`
+     */
+    string queryBuilderMethods() {
+      result =
+        [
+          "select", "addSelect", "where", "andWhere", "orWhere", "having", "orHaving", "andHaving",
+          "orderBy", "addOrderBy", "distinctOn", "groupBy", "addCommonTableExpression",
+          "leftJoinAndSelect", "innerJoinAndSelect", "leftJoin", "innerJoin", "leftJoinAndMapOne",
+          "innerJoinAndMapOne", "leftJoinAndMapMany", "innerJoinAndMapMany", "orUpdate", "orIgnore",
+          "values", "set"
+        ]
+    }
+
+    /**
+     * Gets function names that the return values of these functions can be the results of a database query run
+     */
+    string queryBuilderResult() {
+      result = ["getOne", "getOneOrFail", "getMany", "getRawOne", "getRawMany", "stream"]
+    }
+
+    /**
+     * Gets a QueryBuilder instance that has a query builder function
+     */
+    API::Node getASuccessorOfBuilderInstance(string queryBuilderMethod) {
+      result.getMember(queryBuilderMethod) = queryBuilderInstance().getASuccessor*()
+    }
+
+    /**
+     *  A call to some successor functions of TypeORM `createQueryBuilder` function which are dangerous
+     */
+    private class QueryBuilderCall extends DatabaseAccess, DataFlow::Node {
+      API::Node queryBuilder;
+
+      QueryBuilderCall() {
+        queryBuilder = getASuccessorOfBuilderInstance(queryBuilderMethods()) and
+        this = queryBuilder.asSource()
+      }
+
+      override DataFlow::Node getAResult() {
+        result = queryBuilder.getMember(queryBuilderResult()).getReturn().asSource()
+      }
+
+      override DataFlow::Node getAQueryArgument() {
+        exists(string memberName | memberName = queryBuilderMethods() |
+          memberName = ["leftJoinAndSelect", "innerJoinAndSelect", "leftJoin", "innerJoin"] and
+          result = queryBuilder.getMember(memberName).getParameter(2).asSink()
+          or
+          memberName =
+            ["leftJoinAndMapOne", "innerJoinAndMapOne", "leftJoinAndMapMany", "innerJoinAndMapMany"] and
+          result = queryBuilder.getMember(memberName).getParameter(3).asSink()
+          or
+          memberName =
+            [
+              "select", "addSelect", "where", "andWhere", "orWhere", "having", "orHaving",
+              "andHaving", "orderBy", "addOrderBy", "distinctOn", "groupBy",
+              "addCommonTableExpression"
+            ] and
+          result = queryBuilder.getMember(memberName).getParameter(0).asSink()
+          or
+          memberName = ["orIgnore", "orUpdate"] and
+          result = queryBuilder.getMember(memberName).getParameter([0, 1]).asSink()
+          or
+          // following functions if use a function as their input fields,called function parameters which are vulnerable
+          memberName = ["values", "set"] and
+          result =
+            queryBuilder.getMember(memberName).getParameter(0).getAMember().getReturn().asSink()
+        )
+      }
+    }
+
+    /**
+     *  A call to the TypeORM `query` function of a `QueryRunner`
+     */
+    private class QueryRunner extends DatabaseAccess, API::CallNode {
+      QueryRunner() { queryRunner().getMember("query").getACall() = this }
+
+      override DataFlow::Node getAResult() { result = this }
+
+      override DataFlow::Node getAQueryArgument() { result = this.getArgument(0) }
+    }
+
+    /** An expression that is passed to the `query` function and hence interpreted as SQL. */
+    class QueryString extends SQL::SqlString {
+      QueryString() {
+        this = any(QueryRunner qr).getAQueryArgument() or
+        this = any(QueryBuilderCall qb).getAQueryArgument()
+      }
+    }
+  }
+}

javascript/ql/src/meta/analysis-quality/UnmodelledSteps.ql

@@ -9,7 +9,6 @@
  */
 
 import javascript
-import meta.MetaMetrics
 private import Expressions.ExprHasNoEffect
 import meta.internal.TaintMetrics
 

javascript/ql/src/qlpack.yml

@@ -1,6 +1,6 @@
 name: codeql/javascript-queries
-version: 0.6.3-dev
-groups: 
+version: 0.8.7-dev
+groups:
   - javascript
   - queries
 suites: codeql-suites