Merge branch 'js-team-sprint' into build-leaks

2025-12-20 18:56:32 +01:00 · 2020-06-17 19:51:30 +02:00
parent 69888f90c6 6d6f29eb85
commit bdda587247
69 changed files with 2135 additions and 75 deletions
--- a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.qhelp
+++ b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.qhelp
@@ -0,0 +1,38 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+  <overview>
+    <p>
+        Using string concatenation to construct JavaScript code can be error-prone, or in the worst
+        case, enable code injection if an input is constructed by an attacker.  
+    </p>
+  </overview>
+
+  <recommendation>
+    <p>
+      If using <code>JSON.stringify</code> or a HTML sanitizer to sanitize a string inserted into 
+      JavaScript code, then make sure to perform additional sanitization or remove potentially
+      dangerous characters. 
+    </p>
+  </recommendation>
+
+  <example>
+    <p>
+      The example below constructs a function that assigns the number 42 to the property <code>key</code>
+      on an object <code>obj</code>. However, if <code>key</code> contains <code>&lt;/script&gt;</code>, then 
+      the generated code will break out of a <code>&lt;/script&gt;</code> if inserted into a 
+      <code>&lt;/script&gt;</code> tag.
+    </p>
+    <sample src="examples/ImproperCodeSanitization.js" />
+    <p>
+      The issue has been fixed by escaping potentially dangerous characters, as shown below.
+    </p>
+    <sample src="examples/ImproperCodeSanitizationFixed.js" />
+  </example>
+
+  <references>
+    <li>OWASP: <a href="https://www.owasp.org/index.php/Code_Injection">Code Injection</a>.</li>
+  </references>
+</qhelp>
--- a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
+++ b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
@@ -0,0 +1,71 @@
+/**
+ * @name Improper code sanitization
+ * @description Escaping code as HTML does not provide protection against code injection.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id js/bad-code-sanitization
+ * @tags security
+ *       external/cwe/cwe-094
+ *       external/cwe/cwe-079
+ *       external/cwe/cwe-116
+ */
+
+import javascript
+import semmle.javascript.security.dataflow.ImproperCodeSanitization::ImproperCodeSanitization
+import DataFlow::PathGraph
+private import semmle.javascript.heuristics.HeuristicSinks
+private import semmle.javascript.security.dataflow.CodeInjectionCustomizations
+
+/**
+ * Gets a type-tracked instance of `RemoteFlowSource` using type-tracker `t`.
+ */
+private DataFlow::Node remoteFlow(DataFlow::TypeTracker t) {
+  t.start() and
+  result instanceof RemoteFlowSource
+  or
+  exists(DataFlow::TypeTracker t2, DataFlow::Node prev | prev = remoteFlow(t2) |
+    t2 = t.smallstep(prev, result)
+    or
+    any(TaintTracking::AdditionalTaintStep dts).step(prev, result) and
+    t = t2
+  )
+}
+
+/**
+ * Gets a type-tracked reference to a `RemoteFlowSource`.
+ */
+private DataFlow::Node remoteFlow() { result = remoteFlow(DataFlow::TypeTracker::end()) }
+
+/**
+ * Gets a type-back-tracked instance of a code injection sink using type-tracker `t`.
+ */
+private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) {
+  t.start() and
+  (
+    result instanceof CodeInjection::Sink
+    or
+    result instanceof HeuristicCodeInjectionSink and
+    not result instanceof StringOps::ConcatenationRoot // the heuristic CodeInjection sink looks for string-concats, we are not interrested in those here.
+  )
+  or
+  exists(DataFlow::TypeBackTracker t2 | t = t2.smallstep(result, endsInCodeInjectionSink(t2)))
+}
+
+/**
+ * Gets a reference to to a data-flow node that ends in a code injection sink.
+ */
+private DataFlow::Node endsInCodeInjectionSink() {
+  result = endsInCodeInjectionSink(DataFlow::TypeBackTracker::end())
+}
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where
+  cfg.hasFlowPath(source, sink) and
+  // Basic detection of duplicate results with `js/code-injection`.
+  not (
+    sink.getNode().(StringOps::ConcatenationLeaf).getRoot() = endsInCodeInjectionSink() and
+    remoteFlow() = source.getNode().(DataFlow::InvokeNode).getAnArgument()
+  )
+select sink.getNode(), source, sink, "$@ flows to here and is used to construct code.",
+  source.getNode(), "Improperly sanitized value"
--- a/javascript/ql/src/Security/CWE-094/examples/ImproperCodeSanitization.js
+++ b/javascript/ql/src/Security/CWE-094/examples/ImproperCodeSanitization.js
@@ -0,0 +1,4 @@
+function createObjectWrite() {
+    const assignment = `obj[${JSON.stringify(key)}]=42`;
+    return `(function(){${assignment}})` // NOT OK
+}
--- a/javascript/ql/src/Security/CWE-094/examples/ImproperCodeSanitizationFixed.js
+++ b/javascript/ql/src/Security/CWE-094/examples/ImproperCodeSanitizationFixed.js
@@ -0,0 +1,23 @@
+const charMap = {
+    '<': '\\u003C',
+    '>' : '\\u003E',
+    '/': '\\u002F',
+    '\\': '\\\\',
+    '\b': '\\b',
+    '\f': '\\f',
+    '\n': '\\n',
+    '\r': '\\r',
+    '\t': '\\t',
+    '\0': '\\0',
+    '\u2028': '\\u2028',
+    '\u2029': '\\u2029'
+};
+
+function escapeUnsafeChars(str) {
+    return str.replace(/[<>\b\f\n\r\t\0\u2028\u2029]/g, x => charMap[x])
+}
+
+function createObjectWrite() {
+    const assignment = `obj[${escapeUnsafeChars(JSON.stringify(key))}]=42`;
+    return `(function(){${assignment}})` // OK
+}
--- a/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.qhelp
+++ b/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.qhelp
@@ -0,0 +1,24 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+	<overview>
+
+	</overview>
+
+	<recommendation>
+
+	</recommendation>
+
+	<example>
+
+	</example>
+
+	<references>
+
+		<li>OWASP Top 10: <a href="https://www.owasp.org/index.php/Top_10-2017_A1-Injection">A1 Injection</a>.</li>
+
+	</references>
+
+</qhelp>
--- a/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.ql
+++ b/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.ql
@@ -0,0 +1,81 @@
+/**
+ * @name Incomplete multi-character sanitization
+ * @description A sanitizer that removes a sequence of characters may reintroduce the dangerous sequence.
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id js/incomplete-multi-character-sanitization
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-116
+ *       external/cwe/cwe-20
+ */
+
+import javascript
+import semmle.javascript.security.IncompleteBlacklistSanitizer
+
+predicate isDangerous(RegExpTerm t) {
+  // path traversals
+  t.getAMatchedString() = ["..", "/..", "../"]
+  or
+  exists(RegExpTerm start |
+    start = t.(RegExpSequence).getAChild() and
+    start.getConstantValue() = "." and
+    start.getSuccessor().getConstantValue() = "." and
+    not [start.getPredecessor(), start.getSuccessor().getSuccessor()].getConstantValue() = "."
+  )
+  or
+  // HTML comments
+  t.getAMatchedString() = "<!--"
+  or
+  // HTML scripts
+  t.getAMatchedString().regexpMatch("(?i)<script.*")
+  or
+  exists(RegExpSequence seq | seq = t |
+    t.getChild(0).getConstantValue() = "<" and
+    // the `cript|scrip` case has been observed in the wild, not sure what the goal of that pattern is...
+    t
+        .getChild(0)
+        .getSuccessor+()
+        .getAMatchedString()
+        .regexpMatch("(?i)iframe|script|cript|scrip|style")
+  )
+  or
+  // HTML attributes
+  exists(string dangerousPrefix | dangerousPrefix = ["ng-", "on"] |
+    t.getAMatchedString().regexpMatch("(i?)" + dangerousPrefix + "[a-z]+")
+    or
+    exists(RegExpTerm start, RegExpTerm event | start = t.getAChild() |
+      start.getConstantValue().regexpMatch("(?i)[^a-z]*" + dangerousPrefix) and
+      event = start.getSuccessor() and
+      exists(RegExpTerm quantified | quantified = event.(RegExpQuantifier).getChild(0) |
+        quantified
+            .(RegExpCharacterClass)
+            .getAChild()
+            .(RegExpCharacterRange)
+            .isRange(["a", "A"], ["z", "Z"]) or
+        [quantified, quantified.(RegExpRange).getAChild()].(RegExpCharacterClassEscape).getValue() =
+          "w"
+      )
+    )
+  )
+}
+
+from StringReplaceCall replace, RegExpTerm regexp, RegExpTerm dangerous
+where
+  [replace.getRawReplacement(), replace.getCallback(1).getAReturn()].mayHaveStringValue("") and
+  replace.isGlobal() and
+  regexp = replace.getRegExp().getRoot() and
+  dangerous.getRootTerm() = regexp and
+  isDangerous(dangerous) and
+  // avoid anchored terms
+  not exists(RegExpAnchor a | a.getRootTerm() = regexp) and
+  // avoid flagging wrappers
+  not (
+    dangerous instanceof RegExpAlt or
+    dangerous instanceof RegExpGroup
+  ) and
+  // don't flag replace operations in a loop
+  not replace.getReceiver().getALocalSource() = replace
+select replace, "The replaced string may still contain a substring that starts matching at $@.",
+  dangerous, dangerous.toString()
--- a/javascript/ql/src/Security/CWE-200/PrivateFileExposure.qhelp
+++ b/javascript/ql/src/Security/CWE-200/PrivateFileExposure.qhelp
@@ -0,0 +1,27 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+  <p>
+    Placeholder
+  </p>
+</overview>
+
+<recommendation>
+  <p>
+    Placeholder
+  </p>
+</recommendation>
+
+<example>
+  <p>
+    Placeholder
+  </p>
+</example>
+
+<references>
+  <li>OWASP: <a href="https://www.owasp.org/index.php/Top_10-2017_A3-Sensitive_Data_Exposure">Sensitive Data Exposure</a>.</li>
+</references>
+</qhelp>
--- a/javascript/ql/src/Security/CWE-200/PrivateFileExposure.ql
+++ b/javascript/ql/src/Security/CWE-200/PrivateFileExposure.ql
@@ -0,0 +1,116 @@
+/**
+ * @name Exposure of private files
+ * @description Exposing a node_modules folder, or the project folder to the public, can cause exposure
+ *              of private information.
+ * @kind problem
+ * @problem.severity warning
+ * @id js/exposure-of-private-files
+ * @tags security
+ *       external/cwe/cwe-200
+ * @precision high
+ */
+
+import javascript
+
+/**
+ * Holds if `folder` is a node_modules folder, and at most 1 subdirectory down.
+ */
+bindingset[folder]
+predicate isNodeModuleFolder(string folder) {
+  folder.regexpMatch("(\\.?\\.?/)*node_modules(/|(/[a-zA-Z@_-]+/?))?")
+}
+
+/**
+ * Get a data-flow node that represents a path to the node_modules folder represented by the string-literal `path`.
+ */
+DataFlow::Node getANodeModulePath(string path) {
+  result.getStringValue() = path and
+  isNodeModuleFolder(path)
+  or
+  exists(DataFlow::CallNode resolve |
+    resolve = DataFlow::moduleMember("path", ["resolve", "join"]).getACall()
+  |
+    result = resolve and
+    resolve.getLastArgument() = getANodeModulePath(path)
+  )
+  or
+  exists(StringOps::ConcatenationRoot root | root = result |
+    root.getLastLeaf() = getANodeModulePath(path)
+  )
+  or
+  result.getAPredecessor() = getANodeModulePath(path) // local data-flow
+  or
+  exists(string base, string folder |
+    path = base + folder and
+    folder.regexpMatch("(/)?[a-zA-Z@_-]+/?") and
+    base.regexpMatch("(\\.?\\.?/)*node_modules(/)?") // node_modules, without any sub-folders.
+  |
+    exists(StringOps::ConcatenationRoot root | root = result |
+      root.getNumOperand() = 2 and
+      root.getFirstLeaf() = getANodeModulePath(base) and
+      root.getLastLeaf().getStringValue() = folder
+    )
+    or
+    exists(DataFlow::CallNode resolve |
+      resolve = DataFlow::moduleMember("path", ["resolve", "join"]).getACall()
+    |
+      result = resolve and
+      resolve.getNumArgument() = 2 and
+      resolve.getArgument(0) = getANodeModulePath(path) and
+      resolve.getArgument(1).mayHaveStringValue(folder)
+    )
+  )
+}
+
+/**
+ * Gets a folder that contains a `package.json` file.
+ */
+pragma[noinline]
+Folder getAPackageJSONFolder() { result = any(PackageJSON json).getFile().getParentContainer() }
+
+/**
+ * Gets a reference to `dirname` that might cause information to be leaked.
+ * That can happen if there is a `package.json` file in the same folder.
+ * (It is assumed that the presence of a `package.json` file means that a `node_modules` folder can also exist.
+ */
+DataFlow::Node dirname() {
+  exists(ModuleScope ms | result.asExpr() = ms.getVariable("__dirname").getAnAccess()) and
+  result.getFile().getParentContainer() = getAPackageJSONFolder()
+  or
+  result.getAPredecessor() = dirname()
+  or
+  exists(StringOps::ConcatenationRoot root | root = result |
+    root.getNumOperand() = 2 and
+    root.getOperand(0) = dirname() and
+    root.getOperand(1).getStringValue() = "/"
+  )
+}
+
+/**
+ * Gets a data-flow node that represents a path to the private folder `path`.
+ */
+DataFlow::Node getAPrivateFolderPath(string description) {
+  exists(string path |
+    result = getANodeModulePath(path) and description = "the folder \"" + path + "\""
+  )
+  or
+  result = dirname() and
+  description = "the folder " + result.getFile().getParentContainer().getRelativePath()
+  or
+  result.getStringValue() = [".", "./"] and
+  description = "the current working folder"
+}
+
+/**
+ * Gest a call that serves the folder `path` to the public.
+ */
+DataFlow::CallNode servesAPrivateFolder(string description) {
+  result = DataFlow::moduleMember("express", "static").getACall() and
+  result.getArgument(0) = getAPrivateFolderPath(description)
+}
+
+from Express::RouteSetup setup, string path
+where
+  setup.isUseCall() and
+  setup.getArgument([0 .. 1]) = servesAPrivateFolder(path).getEnclosingExpr()
+select setup, "Serves " + path + ", which can contain private information."
--- a/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.qhelp
+++ b/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.qhelp
@@ -0,0 +1,22 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+	<overview>
+
+	</overview>
+
+	<recommendation>
+
+	</recommendation>
+
+	<example>
+
+	</example>
+
+	<references>
+
+	</references>
+
+</qhelp>
--- a/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql
+++ b/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql
@@ -0,0 +1,40 @@
+/**
+ * @name Disabling certificate validation
+ * @description Disabling cryptographic certificate validation can cause security vulnerabilities.
+ * @kind problem
+ * @problem.severity error
+ * @precision very-high
+ * @id js/disabling-certificate-validation
+ * @tags security
+ *       external/cwe-295
+ */
+
+import javascript
+
+from DataFlow::PropWrite disable
+where
+  exists(DataFlow::SourceNode env |
+    env = NodeJSLib::process().getAPropertyRead("env") and
+    disable = env.getAPropertyWrite("NODE_TLS_REJECT_UNAUTHORIZED") and
+    disable.getRhs().mayHaveStringValue("0")
+  )
+  or
+  exists(DataFlow::ObjectLiteralNode options, DataFlow::InvokeNode invk |
+    options.flowsTo(invk.getAnArgument()) and
+    disable = options.getAPropertyWrite("rejectUnauthorized") and
+    disable.getRhs().(AnalyzedNode).getTheBooleanValue() = false
+  |
+    invk instanceof NodeJSLib::NodeJSClientRequest
+    or
+    invk = DataFlow::moduleMember("https", "Agent").getAnInstantiation()
+    or
+    exists(DataFlow::NewNode new |
+      new = DataFlow::moduleMember("tls", "TLSSocket").getAnInstantiation()
+    |
+      invk = new or
+      invk = new.getAMethodCall("renegotiate")
+    )
+    or
+    invk = DataFlow::moduleMember("tls", ["connect", "createServer"]).getACall()
+  )
+select disable, "Disabling certificate validation is strongly discouraged."
--- a/javascript/ql/src/Security/CWE-327/BadRandomness.qhelp
+++ b/javascript/ql/src/Security/CWE-327/BadRandomness.qhelp
@@ -0,0 +1,36 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+	<overview>
+		<p>
+			Placeholder
+		</p>
+
+	</overview>
+	<recommendation>
+
+		<p>
+			Placeholder.
+		</p>
+
+	</recommendation>
+	<example>
+
+		<p>
+			Placeholder
+		</p>
+
+	</example>
+
+	<references>
+		<li>NIST, FIPS 140 Annex a: <a href="http://csrc.nist.gov/publications/fips/fips140-2/fips1402annexa.pdf"> Approved Security Functions</a>.</li>
+		<li>NIST, SP 800-131A: <a href="http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar1.pdf"> Transitions: Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Lengths</a>.</li>
+		<li>OWASP: <a
+		href="https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html#rule---use-strong-approved-authenticated-encryption">Rule
+		- Use strong approved cryptographic algorithms</a>.
+		</li>
+		<li>Stack Overflow: <a href="https://stackoverflow.com/questions/3956478/understanding-randomness">Understanding “randomness”</a>.</li>
+	</references>
+
+</qhelp>
--- a/javascript/ql/src/Security/CWE-327/BadRandomness.ql
+++ b/javascript/ql/src/Security/CWE-327/BadRandomness.ql
@@ -0,0 +1,195 @@
+/**
+ * @name Creating biased random numbers from cryptographically secure source.
+ * @description Some mathematical operations on random numbers can cause bias in
+ *              the results and compromise security.
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id js/biased-cryptographic-random
+ * @tags security
+ *       external/cwe/cwe-327
+ */
+
+import javascript
+private import semmle.javascript.dataflow.internal.StepSummary
+private import semmle.javascript.security.dataflow.InsecureRandomnessCustomizations
+private import semmle.javascript.dataflow.InferredTypes
+
+/**
+ * Gets a number that is a power of 2.
+ */
+private int powerOfTwo() {
+  result = 1
+  or
+  result = 2 * powerOfTwo() and
+  not result < 0
+}
+
+/**
+ * Gets a node that has value 2^n for some n.
+ */
+private DataFlow::Node isPowerOfTwo() {
+  exists(DataFlow::Node prev |
+    prev.getIntValue() = powerOfTwo()
+    or
+    // Getting around the 32 bit ints in QL. These are some hex values of the form 0x10000000
+    prev.asExpr().(NumberLiteral).getValue() =
+      ["281474976710656", "17592186044416", "1099511627776", "68719476736", "4294967296"]
+  |
+    result = prev.getASuccessor*()
+  )
+}
+
+/**
+ * Gets a node that has value (2^n)-1 for some n.
+ */
+private DataFlow::Node isPowerOfTwoMinusOne() {
+  exists(DataFlow::Node prev |
+    prev.getIntValue() = powerOfTwo() - 1
+    or
+    // Getting around the 32 bit ints in QL. These are some hex values of the form 0xfffffff
+    prev.asExpr().(NumberLiteral).getValue() =
+      ["281474976710655", "17592186044415", "1099511627775", "68719476735", "4294967295"]
+  |
+    result = prev.getASuccessor*()
+  )
+}
+
+/**
+ * Gets a Buffer/TypedArray containing cryptographically secure random numbers.
+ */
+private DataFlow::SourceNode randomBufferSource() {
+  result = DataFlow::moduleMember("crypto", ["randomBytes", "randomFillSync"]).getACall()
+  or
+  exists(DataFlow::CallNode call |
+    call = DataFlow::moduleMember("crypto", ["randomFill", "randomFillSync"]) and
+    result = call.getArgument(0).getALocalSource()
+  )
+  or
+  result = DataFlow::globalVarRef("crypto").getAMethodCall("getRandomValues")
+  or
+  result = DataFlow::moduleImport("secure-random").getACall()
+  or
+  result =
+    DataFlow::moduleImport("secure-random")
+        .getAMethodCall(["randomArray", "randomUint8Array", "randomBuffer"])
+}
+
+/**
+ * Gets the pseudo-property used to track elements inside a Buffer.
+ * The API for `Set` is close enough to the API for `Buffer` that we can reuse the type-tracking steps.
+ */
+private string prop() { result = DataFlow::PseudoProperties::setElement() }
+
+/**
+ * Gets a reference to a cryptographically secure random number produced by `source` and type tracked using `t`.
+ */
+private DataFlow::Node goodRandom(DataFlow::TypeTracker t, DataFlow::SourceNode source) {
+  t.startInProp(prop()) and
+  result = randomBufferSource() and
+  result = source
+  or
+  // Loading a number from a `Buffer`.
+  exists(DataFlow::TypeTracker t2 | t = t2.append(LoadStep(prop())) |
+    // the random generators return arrays/Buffers of random numbers, we therefore track through an indexed read.
+    exists(DataFlow::PropRead read | result = read |
+      read.getBase() = goodRandom(t2, source) and
+      not read.getPropertyNameExpr() instanceof Label
+    )
+    or
+    // reading a number from a Buffer.
+    exists(DataFlow::MethodCallNode call | result = call |
+      call.getReceiver() = goodRandom(t2, source) and
+      call
+          .getMethodName()
+          .regexpMatch("read(BigInt|BigUInt|Double|Float|Int|UInt)(8|16|32|64)?(BE|LE)?")
+    )
+  )
+  or
+  exists(DataFlow::TypeTracker t2 | t = t2.smallstep(goodRandom(t2, source), result))
+  or
+  // re-using the collection steps for `Set`.
+  exists(DataFlow::TypeTracker t2 |
+    result = CollectionsTypeTracking::collectionStep(goodRandom(t2, source), t, t2)
+  )
+  or
+  InsecureRandomness::isAdditionalTaintStep(goodRandom(t.continue(), source), result) and
+  // bit shifts and multiplication by powers of two are generally used for constructing larger numbers from smaller numbers.
+  not exists(BinaryExpr binop | binop = result.asExpr() |
+    binop.getOperator().regexpMatch(".*(<|>).*")
+    or
+    binop.getOperator() = "*" and isPowerOfTwo().asExpr() = binop.getAnOperand()
+    or
+    // string concat does not produce a number
+    unique(InferredType type | type = binop.flow().analyze().getAType()) = TTString()
+  )
+}
+
+/**
+ * Gets a reference to a cryptographically secure random number produced by `source`.
+ */
+DataFlow::Node goodRandom(DataFlow::SourceNode source) {
+  result = goodRandom(DataFlow::TypeTracker::end(), source)
+}
+
+/**
+ * Gets a node that that produces a biased result from otherwise cryptographically secure random numbers produced by `source`.
+ */
+DataFlow::Node badCrypto(string description, DataFlow::SourceNode source) {
+  // addition and multiplication - always bad when both the lhs and rhs are random.
+  exists(BinaryExpr binop | result.asExpr() = binop |
+    goodRandom(_).asExpr() = binop.getLeftOperand() and
+    goodRandom(_).asExpr() = binop.getRightOperand() and
+    goodRandom(source).asExpr() = binop.getAnOperand() and
+    (
+      binop.getOperator() = "+" and description = "addition"
+      or
+      binop.getOperator() = "*" and description = "multiplication"
+    )
+  )
+  or
+  // division - bad if result is rounded.
+  exists(DivExpr div | result.asExpr() = div |
+    goodRandom(source).asExpr() = div.getLeftOperand() and
+    description = "division and rounding the result" and
+    not div.getRightOperand() = isPowerOfTwoMinusOne().asExpr() and // division by (2^n)-1 most of the time produces a uniformly random number between 0 and 1.
+    DataFlow::globalVarRef("Math")
+        .getAMemberCall(["round", "floor", "ceil"])
+        .getArgument(0)
+        .asExpr() = div
+  )
+  or
+  // modulo - only bad if not by a power of 2 - and the result is not checked for bias
+  exists(ModExpr mod, DataFlow::Node random | result.asExpr() = mod and mod.getOperator() = "%" |
+    description = "modulo" and
+    goodRandom(source) = random and
+    random.asExpr() = mod.getLeftOperand() and
+    // division by a power of 2 is OK. E.g. if `x` is uniformly random is in the range [0..255] then `x % 32` is uniformly random in the range [0..31].
+    not mod.getRightOperand() = isPowerOfTwo().asExpr() and
+    // not exists a comparison that checks if the result is potentially biased.
+    not exists(BinaryExpr comparison | comparison.getOperator() = [">", "<", "<=", ">="] |
+      AccessPath::getAnAliasedSourceNode(random.getALocalSource())
+          .flowsToExpr(comparison.getAnOperand())
+      or
+      exists(DataFlow::PropRead otherRead |
+        otherRead = random.(DataFlow::PropRead).getBase().getALocalSource().getAPropertyRead() and
+        not exists(otherRead.getPropertyName()) and
+        otherRead.flowsToExpr(comparison.getAnOperand())
+      )
+    )
+  )
+  or
+  // create a number from a string - always a bad idea.
+  exists(DataFlow::CallNode number, StringOps::ConcatenationRoot root | result = number |
+    number = DataFlow::globalVarRef(["Number", "parseInt", "parseFloat"]).getACall() and
+    root = number.getArgument(0) and
+    goodRandom(source) = root.getALeaf() and
+    exists(root.getALeaf().getStringValue()) and
+    description = "string concatenation"
+  )
+}
+
+from DataFlow::Node node, string description, DataFlow::SourceNode source
+where node = badCrypto(description, source)
+select node, "Using " + description + " on a $@ produces biased results.", source,
+  "cryptographically secure random number"
--- a/javascript/ql/src/Security/CWE-730/ServerCrash.qhelp
+++ b/javascript/ql/src/Security/CWE-730/ServerCrash.qhelp
@@ -0,0 +1,22 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+	<overview>
+
+	</overview>
+
+	<recommendation>
+
+	</recommendation>
+
+	<example>
+
+	</example>
+
+	<references>
+
+	</references>
+
+</qhelp>
--- a/javascript/ql/src/Security/CWE-730/ServerCrash.ql
+++ b/javascript/ql/src/Security/CWE-730/ServerCrash.ql
@@ -0,0 +1,100 @@
+/**
+ * @name Server crash
+ * @description A server that can be forced to crash may be vulnerable to denial-of-service
+ *              attacks.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @id js/server-crash
+ * @tags security
+ *       external/cwe/cwe-730
+ */
+
+import javascript
+
+/**
+ * Gets a function that `caller` invokes.
+ */
+Function getACallee(Function caller) {
+  exists(DataFlow::InvokeNode invk |
+    invk.getEnclosingFunction() = caller and result = invk.getACallee()
+  )
+}
+
+/**
+ * Gets a function that `caller` invokes, excluding calls guarded in `try`-blocks.
+ */
+Function getAnUnguardedCallee(Function caller) {
+  exists(DataFlow::InvokeNode invk |
+    invk.getEnclosingFunction() = caller and
+    result = invk.getACallee() and
+    not exists(invk.asExpr().getEnclosingStmt().getEnclosingTryCatchStmt())
+  )
+}
+
+predicate isHeaderValue(HTTP::ExplicitHeaderDefinition def, DataFlow::Node node) {
+  def.definesExplicitly(_, node.asExpr())
+}
+
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "Configuration" }
+
+  override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node node) {
+    // using control characters in a header value will cause an exception
+    isHeaderValue(_, node)
+  }
+}
+
+predicate isLikelyToThrow(DataFlow::Node crash) {
+  exists(Configuration cfg, DataFlow::Node sink | cfg.hasFlow(_, sink) | isHeaderValue(crash, sink))
+}
+
+/**
+ * A call that looks like it is asynchronous.
+ */
+class AsyncCall extends DataFlow::CallNode {
+  DataFlow::FunctionNode callback;
+
+  AsyncCall() {
+    callback.flowsTo(getLastArgument()) and
+    callback.getParameter(0).getName() = ["e", "err", "error"] and
+    callback.getNumParameter() = 2 and
+    not exists(callback.getAReturn())
+  }
+
+  DataFlow::FunctionNode getCallback() { result = callback }
+}
+
+/**
+ * Gets a function that is invoked by `asyncCallback` without any try-block wrapping, `asyncCallback` is in turn is called indirectly by `routeHandler`.
+ *
+ * If the result throws an excection, the server of `routeHandler` will crash.
+ */
+Function getAPotentialServerCrasher(
+  HTTP::RouteHandler routeHandler, DataFlow::FunctionNode asyncCallback
+) {
+  exists(AsyncCall asyncCall |
+    // the route handler transitively calls an async function
+    asyncCall.getEnclosingFunction() =
+      getACallee*(routeHandler.(DataFlow::FunctionNode).getFunction()) and
+    asyncCallback = asyncCall.getCallback() and
+    // the async function transitively calls a function that may throw an exception out of the the async function
+    result = getAnUnguardedCallee*(asyncCallback.getFunction())
+  )
+}
+
+/**
+ * Gets an AST node that is likely to throw an uncaught exception in `fun`.
+ */
+ExprOrStmt getALikelyExceptionThrower(Function fun) {
+  result.getContainer() = fun and
+  not exists([result.(Expr).getEnclosingStmt(), result.(Stmt)].getEnclosingTryCatchStmt()) and
+  (isLikelyToThrow(result.(Expr).flow()) or result instanceof ThrowStmt)
+}
+
+from HTTP::RouteHandler routeHandler, DataFlow::FunctionNode asyncCallback, ExprOrStmt crasher
+where crasher = getALikelyExceptionThrower(getAPotentialServerCrasher(routeHandler, asyncCallback))
+select crasher, "When an exception is thrown here and later exits $@, the server of $@ will crash.",
+  asyncCallback, "this asynchronous callback", routeHandler, "this route handler"
--- a/javascript/ql/src/semmle/javascript/DOM.qll
+++ b/javascript/ql/src/semmle/javascript/DOM.qll
@@ -291,11 +291,27 @@ module DOM {
     */
    abstract class Range extends DataFlow::Node { }

+    private string getADomPropertyName() {
+      exists(ExternalInstanceMemberDecl decl |
+        result = decl.getName() and
+        isDomRootType(decl.getDeclaringType().getASupertype*())
+      )
+    }
+
    private class DefaultRange extends Range {
      DefaultRange() {
        this.asExpr().(VarAccess).getVariable() instanceof DOMGlobalVariable
        or
-        this = domValueRef().getAPropertyRead()
+        exists(DataFlow::PropRead read |
+          this = read and
+          read = domValueRef().getAPropertyRead()
+        |
+          not read.mayHavePropertyName(_)
+          or
+          read.mayHavePropertyName(getADomPropertyName())
+          or
+          read.mayHavePropertyName(any(string s | exists(s.toInt())))
+        )
        or
        this = domElementCreationOrQuery()
        or
--- a/javascript/ql/src/semmle/javascript/NodeModuleResolutionImpl.qll
+++ b/javascript/ql/src/semmle/javascript/NodeModuleResolutionImpl.qll
@@ -82,20 +82,20 @@ File tryExtensions(Folder dir, string basename, int priority) {
 * Gets the main module described by `pkg` with the given `priority`.
 */
 File resolveMainModule(PackageJSON pkg, int priority) {
-  if exists(MainModulePath::of(pkg))
-  then
-    exists(PathExpr main | main = MainModulePath::of(pkg) |
-      result = main.resolve() and priority = 0
-      or
-      result = tryExtensions(main.resolve(), "index", priority)
-      or
-      not exists(main.resolve()) and
-      not exists(main.getExtension()) and
-      exists(int n | n = main.getNumComponent() |
-        result = tryExtensions(main.resolveUpTo(n - 1), main.getComponent(n - 1), priority)
-      )
+  exists(PathExpr main | main = MainModulePath::of(pkg) |
+    result = main.resolve() and priority = 0
+    or
+    result = tryExtensions(main.resolve(), "index", priority)
+    or
+    not exists(main.resolve()) and
+    not exists(main.getExtension()) and
+    exists(int n | n = main.getNumComponent() |
+      result = tryExtensions(main.resolveUpTo(n - 1), main.getComponent(n - 1), priority)
    )
-  else result = tryExtensions(pkg.getFile().getParentContainer(), "index", priority)
+  )
+  or
+  result =
+    tryExtensions(pkg.getFile().getParentContainer(), "index", priority - prioritiesPerCandidate())
 }

 /**
--- a/javascript/ql/src/semmle/javascript/PackageExports.qll
+++ b/javascript/ql/src/semmle/javascript/PackageExports.qll
@@ -68,4 +68,9 @@ private DataFlow::Node getAnExportFromModule(Module mod) {
  result.analyze().getAValue() = mod.(NodeModule).getAModuleExportsValue()
  or
  exists(ASTNode export | result.getEnclosingExpr() = export | mod.exports(_, export))
+  or
+  exists(ExportDeclaration export |
+    result = export.getSourceNode(_) and
+    mod = export.getTopLevel()
+  )
 }
--- a/javascript/ql/src/semmle/javascript/Promises.qll
+++ b/javascript/ql/src/semmle/javascript/Promises.qll
@@ -576,6 +576,22 @@ module Bluebird {

    override DataFlow::Node getArrayNode() { result = getArgument(0) }
  }
+
+  /**
+   * An async function created using a call to `bluebird.coroutine`.
+   */
+  class BluebirdCoroutineDefinition extends DataFlow::CallNode {
+    BluebirdCoroutineDefinition() { this = bluebird().getAMemberCall("coroutine") }
+  }
+
+  private class BluebirdCoroutineDefinitionAsPartialInvoke extends DataFlow::PartialInvokeNode::Range,
+    BluebirdCoroutineDefinition {
+    override DataFlow::SourceNode getBoundFunction(DataFlow::Node callback, int boundArgs) {
+      boundArgs = 0 and
+      callback = getArgument(0) and
+      result = this
+    }
+  }
 }

 /**
--- a/javascript/ql/src/semmle/javascript/StringOps.qll
+++ b/javascript/ql/src/semmle/javascript/StringOps.qll
@@ -720,14 +720,8 @@ module StringOps {
      override DataFlow::Node getStringOperand() { result = getArgument(0) }
    }

-    private class MatchesCall extends Range, DataFlow::MethodCallNode {
-      MatchesCall() { getMethodName() = "matches" }
-
-      override DataFlow::Node getRegExpOperand(boolean coerced) {
-        result = getArgument(0) and coerced = true
-      }
-
-      override DataFlow::Node getStringOperand() { result = getReceiver() }
+    private class MatchCall extends DataFlow::MethodCallNode {
+      MatchCall() { getMethodName() = "match" }
    }

    private class ExecCall extends DataFlow::MethodCallNode {
@@ -777,5 +771,22 @@ module StringOps {

      override boolean getPolarity() { result = polarity }
    }
+
+    private class MatchTest extends Range, DataFlow::ValueNode {
+      MatchCall match;
+      boolean polarity;
+
+      MatchTest() {
+        exists(Expr use | match.flowsToExpr(use) | impliesNotNull(astNode, use, polarity))
+      }
+
+      override DataFlow::Node getRegExpOperand(boolean coerced) {
+        result = match.getArgument(0) and coerced = true
+      }
+
+      override DataFlow::Node getStringOperand() { result = match.getReceiver() }
+
+      override boolean getPolarity() { result = polarity }
+    }
  }
 }
--- a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll
@@ -267,6 +267,12 @@ module TaintTracking {
      pred = DataFlow::valueNode(fos.getIterationDomain()) and
      succ = DataFlow::lvalueNode(fos.getLValue())
    )
+    or
+    // taint-tracking rest patterns in l-values. E.g. `const {...spread} = foo()` or `const [...spread] = foo()`.
+    exists(DestructuringPattern pattern |
+      pred = DataFlow::lvalueNode(pattern) and
+      succ = DataFlow::lvalueNode(pattern.getRest())
+    )
  }

  /**
--- a/javascript/ql/src/semmle/javascript/frameworks/HttpFrameworks.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/HttpFrameworks.qll
@@ -2,6 +2,7 @@ import semmle.javascript.frameworks.Express
 import semmle.javascript.frameworks.Hapi
 import semmle.javascript.frameworks.Koa
 import semmle.javascript.frameworks.NodeJSLib
+import semmle.javascript.frameworks.Micro
 import semmle.javascript.frameworks.Restify
 import semmle.javascript.frameworks.Connect
 import semmle.javascript.frameworks.Fastify
--- a/javascript/ql/src/semmle/javascript/frameworks/Micro.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/Micro.qll
@@ -0,0 +1,114 @@
+/**
+ * Provides a model of the `micro` NPM package.
+ */
+
+private import javascript
+
+private module Micro {
+  private import DataFlow
+
+  /**
+   * A node that should be interpreted as a route handler, to use as starting
+   * point for back-tracking.
+   */
+  Node microRouteHandlerSink() {
+    result = moduleMember("micro", "run").getACall().getLastArgument()
+    or
+    result = moduleImport("micro").getACall().getArgument(0)
+  }
+
+  /** Gets a data flow node interpreted as a route handler. */
+  private DataFlow::SourceNode microRouteHandler(DataFlow::TypeBackTracker t) {
+    t.start() and
+    result = microRouteHandlerSink().getALocalSource()
+    or
+    exists(DataFlow::TypeBackTracker t2 | result = microRouteHandler(t2).backtrack(t2, t))
+    or
+    exists(DataFlow::CallNode transformer |
+      transformer = moduleImport("micro-compress").getACall()
+      or
+      transformer instanceof Bluebird::BluebirdCoroutineDefinition
+    |
+      microRouteHandler(t.continue()) = transformer and
+      result = transformer.getArgument(0).getALocalSource()
+    )
+  }
+
+  /** Gets a data flow node interpreted as a route handler. */
+  DataFlow::SourceNode microRouteHandler() {
+    result = microRouteHandler(DataFlow::TypeBackTracker::end())
+  }
+
+  /**
+   * A function passed to `micro` or `micro.run`.
+   */
+  class MicroRouteHandler extends HTTP::Servers::StandardRouteHandler, DataFlow::FunctionNode {
+    MicroRouteHandler() { this = microRouteHandler().getAFunctionValue() }
+  }
+
+  class MicroRequestSource extends HTTP::Servers::RequestSource {
+    MicroRouteHandler h;
+
+    MicroRequestSource() { this = h.getParameter(0) }
+
+    override HTTP::RouteHandler getRouteHandler() { result = h }
+  }
+
+  class MicroResponseSource extends HTTP::Servers::ResponseSource {
+    MicroRouteHandler h;
+
+    MicroResponseSource() { this = h.getParameter(1) }
+
+    override HTTP::RouteHandler getRouteHandler() { result = h }
+  }
+
+  class MicroRequestExpr extends NodeJSLib::RequestExpr {
+    override MicroRequestSource src;
+  }
+
+  class MicroReseponseExpr extends NodeJSLib::ResponseExpr {
+    override MicroResponseSource src;
+  }
+
+  private HTTP::RouteHandler getRouteHandlerFromReqRes(DataFlow::Node node) {
+    exists(HTTP::Servers::RequestSource src |
+      src.ref().flowsTo(node) and
+      result = src.getRouteHandler()
+    )
+    or
+    exists(HTTP::Servers::ResponseSource src |
+      src.ref().flowsTo(node) and
+      result = src.getRouteHandler()
+    )
+  }
+
+  class MicroBodyParserCall extends HTTP::RequestInputAccess, DataFlow::CallNode {
+    string name;
+
+    MicroBodyParserCall() {
+      name = ["buffer", "text", "json"] and
+      this = moduleMember("micro", name).getACall()
+    }
+
+    override string getKind() { result = "body" }
+
+    override HTTP::RouteHandler getRouteHandler() {
+      result = getRouteHandlerFromReqRes(getArgument(0))
+    }
+
+    override predicate isUserControlledObject() { name = "json" }
+  }
+
+  class MicroSendArgument extends HTTP::ResponseSendArgument {
+    CallNode send;
+
+    MicroSendArgument() {
+      send = moduleMember("micro", ["send", "sendError"]).getACall() and
+      this = send.getLastArgument().asExpr()
+    }
+
+    override HTTP::RouteHandler getRouteHandler() {
+      result = getRouteHandlerFromReqRes(send.getArgument([0, 1]))
+    }
+  }
+}
--- a/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll
+++ b/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll
@@ -17,32 +17,12 @@ private import semmle.javascript.security.dataflow.RegExpInjectionCustomizations
 private import semmle.javascript.security.dataflow.ClientSideUrlRedirectCustomizations
 private import semmle.javascript.security.dataflow.ServerSideUrlRedirectCustomizations
 private import semmle.javascript.security.dataflow.InsecureRandomnessCustomizations
+private import HeuristicSinks as Sinks

-/**
- * A heuristic sink for data flow in a security query.
- */
-abstract class HeuristicSink extends DataFlow::Node { }
+class HeuristicSink = Sinks::HeuristicSink;

-private class HeuristicCodeInjectionSink extends HeuristicSink, CodeInjection::Sink {
-  HeuristicCodeInjectionSink() {
-    isAssignedTo(this, "$where")
-    or
-    isAssignedToOrConcatenatedWith(this, "(?i)(command|cmd|exec|code|script|program)")
-    or
-    isArgTo(this, "(?i)(eval|run)") // "exec" clashes too often with `RegExp.prototype.exec`
-    or
-    exists(string srcPattern |
-      // function/lambda syntax anywhere
-      srcPattern = "(?s).*function\\s*\\(.*\\).*" or
-      srcPattern = "(?s).*(\\(.*\\)|[A-Za-z_]+)\\s?=>.*"
-    |
-      isConcatenatedWithString(this, srcPattern)
-    )
-    or
-    // dynamic property name
-    isConcatenatedWithStrings("(?is)[a-z]+\\[", this, "(?s)\\].*")
-  }
-}
+private class HeuristicCodeInjectionSink extends Sinks::HeuristicCodeInjectionSink,
+  CodeInjection::Sink { }

 private class HeuristicCommandInjectionSink extends HeuristicSink, CommandInjection::Sink {
  HeuristicCommandInjectionSink() {
--- a/javascript/ql/src/semmle/javascript/heuristics/HeuristicSinks.qll
+++ b/javascript/ql/src/semmle/javascript/heuristics/HeuristicSinks.qll
@@ -0,0 +1,35 @@
+/**
+ * Provides heuristically recognized sinks for security queries.
+ */
+
+import javascript
+private import SyntacticHeuristics
+
+/**
+ * A heuristic sink for data flow in a security query.
+ */
+abstract class HeuristicSink extends DataFlow::Node { }
+
+/**
+ * A heuristically recognized sink for `js/code-injection` vulnerabilities.
+ */
+class HeuristicCodeInjectionSink extends HeuristicSink {
+  HeuristicCodeInjectionSink() {
+    isAssignedTo(this, "$where")
+    or
+    isAssignedToOrConcatenatedWith(this, "(?i)(command|cmd|exec|code|script|program)")
+    or
+    isArgTo(this, "(?i)(eval|run)") // "exec" clashes too often with `RegExp.prototype.exec`
+    or
+    exists(string srcPattern |
+      // function/lambda syntax anywhere
+      srcPattern = "(?s).*function\\s*\\(.*\\).*" or
+      srcPattern = "(?s).*(\\(.*\\)|[A-Za-z_]+)\\s?=>.*"
+    |
+      isConcatenatedWithString(this, srcPattern)
+    )
+    or
+    // dynamic property name
+    isConcatenatedWithStrings("(?is)[a-z]+\\[", this, "(?s)\\].*")
+  }
+}
--- a/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll
@@ -0,0 +1,30 @@
+/**
+ * Provides a taint-tracking configuration for reasoning about improper code
+ * sanitization.
+ *
+ * Note, for performance reasons: only import this file if
+ * `ImproperCodeSanitization::Configuration` is needed, otherwise
+ * `ImproperCodeSanitizationCustomizations` should be imported instead.
+ */
+
+import javascript
+
+/**
+ * Classes and predicates for reasoning about improper code sanitization.
+ */
+module ImproperCodeSanitization {
+  import ImproperCodeSanitizationCustomizations::ImproperCodeSanitization
+
+  /**
+   * A taint-tracking configuration for reasoning about improper code sanitization vulnerabilities.
+   */
+  class Configuration extends TaintTracking::Configuration {
+    Configuration() { this = "ImproperCodeSanitization" }
+
+    override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+    override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer instanceof Sanitizer }
+  }
+}
--- a/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll
@@ -0,0 +1,68 @@
+/**
+ * Provides default sources, sinks and sanitizers for reasoning about
+ * improper code sanitization, as well as extension points for
+ * adding your own.
+ */
+
+import javascript
+
+/**
+ * Classes and predicates for reasoning about improper code sanitization.
+ */
+module ImproperCodeSanitization {
+  /**
+   * A data flow source for improper code sanitization.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for improper code sanitization.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for improper code sanitization.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /**
+   * A call to a HTML sanitizer seen as a source for improper code sanitization
+   */
+  class HtmlSanitizerCallAsSource extends Source {
+    HtmlSanitizerCallAsSource() { this instanceof HtmlSanitizerCall }
+  }
+
+  /**
+   * A call to `JSON.stringify()` seen as a source for improper code sanitization
+   */
+  class JSONStringifyAsSource extends Source {
+    JSONStringifyAsSource() { this = DataFlow::globalVarRef("JSON").getAMemberCall("stringify") }
+  }
+
+  /**
+   * A leaf in a string-concatenation, where the string-concatenation constructs code that looks like a function.
+   */
+  class FunctionStringConstruction extends Sink, StringOps::ConcatenationLeaf {
+    FunctionStringConstruction() {
+      exists(StringOps::ConcatenationRoot root, int i |
+        root.getOperand(i) = this and
+        not exists(this.getStringValue())
+      |
+        exists(StringOps::ConcatenationLeaf functionLeaf |
+          functionLeaf = root.getOperand(any(int j | j < i))
+        |
+          functionLeaf
+              .getStringValue()
+              .regexpMatch([".*function( )?([a-zA-Z0-9]+)?( )?\\(.*", ".*eval\\(.*",
+                    ".*new Function\\(.*", "(^|.*[^a-zA-Z0-9])\\(.*\\)( )?=>.*"])
+        )
+      )
+    }
+  }
+
+  /**
+   * A call to `String.prototype.replace` seen as a sanitizer for improper code sanitization.
+   * All calls to replace that happens after the initial improper sanitization is seen as a sanitizer.
+   */
+  class StringReplaceCallAsSanitizer extends Sanitizer, StringReplaceCall { }
+}
--- a/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomness.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomness.qll
@@ -36,16 +36,7 @@ module InsecureRandomness {
    }

    override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
-      // Assume that all operations on tainted values preserve taint: crypto is hard
-      succ.asExpr().(BinaryExpr).getAnOperand() = pred.asExpr()
-      or
-      succ.asExpr().(UnaryExpr).getOperand() = pred.asExpr()
-      or
-      exists(DataFlow::MethodCallNode mc |
-        mc = DataFlow::globalVarRef("Math").getAMemberCall(_) and
-        pred = mc.getAnArgument() and
-        succ = mc
-      )
+      InsecureRandomness::isAdditionalTaintStep(pred, succ)
    }
  }
 }
--- a/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomnessCustomizations.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomnessCustomizations.qll
@@ -78,4 +78,20 @@ module InsecureRandomness {
  class CryptoKeySink extends Sink {
    CryptoKeySink() { this instanceof CryptographicKey }
  }
+
+  /**
+   * Holds if the step `pred` -> `succ` is an additional taint-step for random values that are not cryptographically secure.
+   */
+  predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+    // Assume that all operations on tainted values preserve taint: crypto is hard
+    succ.asExpr().(BinaryExpr).getAnOperand() = pred.asExpr()
+    or
+    succ.asExpr().(UnaryExpr).getOperand() = pred.asExpr()
+    or
+    exists(DataFlow::MethodCallNode mc |
+      mc = DataFlow::globalVarRef("Math").getAMemberCall(_) and
+      pred = mc.getAnArgument() and
+      succ = mc
+    )
+  }
 }
--- a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll
@@ -212,11 +212,10 @@ module TaintedPath {
    DataFlow::Node output;

    PreservingPathCall() {
-      exists(string name | name = "dirname" or name = "toNamespacedPath" |
-        this = NodeJSLib::Path::moduleMember(name).getACall() and
-        input = getAnArgument() and
-        output = this
-      )
+      this =
+        NodeJSLib::Path::moduleMember(["dirname", "toNamespacedPath", "parse", "format"]).getACall() and
+      input = getAnArgument() and
+      output = this
      or
      // non-global replace or replace of something other than /\.\./g, /[/]/g, or /[\.]/g.
      this.getCalleeName() = "replace" and