Merge remote-tracking branch 'upstream/master' into exceptionXss

2025-12-21 11:16:30 +01:00 · 2019-11-26 10:54:04 +01:00
parent 7ee12a3420 18e1708036
commit f284b3a2bb
228 changed files with 5669 additions and 1550 deletions
--- a/javascript/ql/src/Expressions/UnboundEventHandlerReceiver.ql
+++ b/javascript/ql/src/Expressions/UnboundEventHandlerReceiver.ql
@@ -14,47 +14,51 @@ import javascript
 * Holds if the receiver of `method` is bound.
 */
 private predicate isBoundInMethod(MethodDeclaration method) {
-  exists(DataFlow::ThisNode thiz, MethodDeclaration bindingMethod |
+  exists(DataFlow::ThisNode thiz, MethodDeclaration bindingMethod, string name |
    bindingMethod.getDeclaringClass() = method.getDeclaringClass() and
    not bindingMethod.isStatic() and
-    thiz.getBinder().getAstNode() = bindingMethod.getBody()
+    thiz.getBinder().getAstNode() = bindingMethod.getBody() and
+    name = method.getName()
  |
+    // binding assignments: `this[x] = <expr>.bind(...)`
    exists(DataFlow::MethodCallNode bind, DataFlow::PropWrite w |
-      // this[x] = <expr>.bind(...)
+      not exists(w.getPropertyName()) or // unknown name, assume everything is bound
+      w.getPropertyName() = name
+    |
      w = thiz.getAPropertyWrite() and
-      not exists(w.getPropertyName()) and
      bind.getMethodName() = "bind" and
      bind.flowsTo(w.getRhs())
    )
    or
-    // require("auto-bind")(this)
+    // library binders
    exists(string mod |
      mod = "auto-bind" or
      mod = "react-autobind"
    |
      thiz.flowsTo(DataFlow::moduleImport(mod).getACall().getArgument(0))
-    )
-    or
-    exists(string name | name = method.getName() |
-      exists(DataFlow::MethodCallNode bind |
-        // this.<methodName> = <expr>.bind(...)
-        bind = thiz.getAPropertySource(name) and
-        bind.getMethodName() = "bind"
+    ) or
+    // heuristic reflective binders
+    exists(DataFlow::CallNode binder, string calleeName |
+      (
+        binder.(DataFlow::MethodCallNode).getMethodName() = calleeName or
+        binder.getCalleeNode().asExpr().(VarAccess).getVariable().getName() = calleeName
+      ) and
+      calleeName.regexpMatch("(?i).*bind.*") and
+      thiz.flowsTo(binder.getAnArgument()) and
+      // exclude the binding assignments
+      not thiz.getAPropertySource() = binder
+    |
+      // `myBindAll(this)`
+      binder.getNumArgument() = 1
+      or
+      // `myBindSome(this, [<name1>, <name2>])`
+      exists(DataFlow::ArrayCreationNode names |
+        names.flowsTo(binder.getAnArgument()) and
+        names.getAnElement().mayHaveStringValue(name)
      )
      or
-      exists(DataFlow::MethodCallNode bindAll |
-        bindAll.getMethodName() = "bindAll" and
-        thiz.flowsTo(bindAll.getArgument(0))
-      |
-        // _.bindAll(this, <name1>)
-        bindAll.getArgument(1).mayHaveStringValue(name)
-        or
-        // _.bindAll(this, [<name1>, <name2>])
-        exists(DataFlow::ArrayCreationNode names |
-          names.flowsTo(bindAll.getArgument(1)) and
-          names.getAnElement().mayHaveStringValue(name)
-        )
-      )
+      // `myBindSome(this, <name1>, <name2>)`
+      binder.getAnArgument().mayHaveStringValue(name)
    )
  )
  or
@@ -66,10 +70,10 @@ private predicate isBoundInMethod(MethodDeclaration method) {
    ) and
    name.regexpMatch("(?i).*(bind|bound).*")
  |
-    // @autobind
+    // `@autobind`
    decoration.(Identifier).getName() = name
    or
-    // @action.bound
+    // `@action.bound`
    decoration.(PropAccess).getPropertyName() = name
  )
 }
--- a/javascript/ql/src/Security/CWE-116/DoubleEscaping.ql
+++ b/javascript/ql/src/Security/CWE-116/DoubleEscaping.ql
@@ -15,38 +15,17 @@

 import javascript

-/**
- * Holds if `rl` is a simple constant, which is bound to the result of the predicate.
- *
- * For example, `/a/g` has string value `"a"` and `/abc/` has string value `"abc"`,
- * while `/ab?/` and `/a(?=b)/` do not have a string value.
- *
- * Flags are ignored, so `/a/i` is still considered to have string value `"a"`,
- * even though it also matches `"A"`.
- *
- * Note the somewhat subtle use of monotonic aggregate semantics, which makes the
- * `strictconcat` fail if one of the children of the root is not a constant (legacy
- * semantics would simply skip such children).
- */
-language[monotonicAggregates]
-string getStringValue(RegExpLiteral rl) {
-  exists(RegExpTerm root | root = rl.getRoot() |
-    result = root.(RegExpConstant).getValue()
-    or
-    result = strictconcat(RegExpTerm ch, int i |
-        ch = root.(RegExpSequence).getChild(i)
-      |
-        ch.(RegExpConstant).getValue() order by i
-      )
-  )
-}
-
 /**
 * Gets a predecessor of `nd` that is not an SSA phi node.
 */
 DataFlow::Node getASimplePredecessor(DataFlow::Node nd) {
  result = nd.getAPredecessor() and
-  not nd.(DataFlow::SsaDefinitionNode).getSsaVariable().getDefinition() instanceof SsaPhiNode
+  not exists(SsaDefinition ssa |
+    ssa = nd.(DataFlow::SsaDefinitionNode).getSsaVariable().getDefinition()
+  |
+    ssa instanceof SsaPhiNode or
+    ssa instanceof SsaVariableCapture
+  )
 }

 /**
@@ -54,37 +33,33 @@ DataFlow::Node getASimplePredecessor(DataFlow::Node nd) {
 * into a form described by regular expression `regex`.
 */
 predicate escapingScheme(string metachar, string regex) {
-  metachar = "&" and regex = "&.*;"
+  metachar = "&" and regex = "&.+;"
  or
-  metachar = "%" and regex = "%.*"
+  metachar = "%" and regex = "%.+"
  or
-  metachar = "\\" and regex = "\\\\.*"
+  metachar = "\\" and regex = "\\\\.+"
 }

 /**
 * A call to `String.prototype.replace` that replaces all instances of a pattern.
 */
-class Replacement extends DataFlow::Node {
-  RegExpLiteral pattern;
-
+class Replacement extends StringReplaceCall {
  Replacement() {
-    exists(DataFlow::MethodCallNode mcn | this = mcn |
-      mcn.getMethodName() = "replace" and
-      pattern.flow().(DataFlow::SourceNode).flowsTo(mcn.getArgument(0)) and
-      mcn.getNumArgument() = 2 and
-      pattern.isGlobal()
-    )
+    isGlobal()
  }

  /**
-   * Holds if this replacement replaces the string `input` with `output`.
+   * Gets the input of this replacement.
   */
-  predicate replaces(string input, string output) {
-    exists(DataFlow::MethodCallNode mcn |
-      mcn = this and
-      input = getStringValue(pattern) and
-      output = mcn.getArgument(1).getStringValue()
-    )
+  DataFlow::Node getInput() {
+    result = this.getReceiver()
+  }
+
+  /**
+   * Gets the output of this replacement.
+   */
+  DataFlow::SourceNode getOutput() {
+    result = this
  }

  /**
@@ -119,7 +94,7 @@ class Replacement extends DataFlow::Node {
   * Gets the previous replacement in this chain of replacements.
   */
  Replacement getPreviousReplacement() {
-    result = getASimplePredecessor*(this.(DataFlow::MethodCallNode).getReceiver())
+    result.getOutput() = getASimplePredecessor*(getInput())
  }

  /**
@@ -130,7 +105,9 @@ class Replacement extends DataFlow::Node {
    exists(Replacement pred | pred = this.getPreviousReplacement() |
      if pred.escapes(_, metachar)
      then result = pred
-      else result = pred.getAnEarlierEscaping(metachar)
+      else (
+        not pred.unescapes(metachar, _) and result = pred.getAnEarlierEscaping(metachar)
+      )
    )
  }

@@ -142,7 +119,9 @@ class Replacement extends DataFlow::Node {
    exists(Replacement succ | this = succ.getPreviousReplacement() |
      if succ.unescapes(metachar, _)
      then result = succ
-      else result = succ.getALaterUnescaping(metachar)
+      else (
+        not succ.escapes(_, metachar) and result = succ.getALaterUnescaping(metachar)
+      )
    )
  }
 }
--- a/javascript/ql/src/Security/CWE-116/IncompleteSanitization.qhelp
+++ b/javascript/ql/src/Security/CWE-116/IncompleteSanitization.qhelp
@@ -35,6 +35,18 @@ likely to handle corner cases correctly than a custom implementation.
 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>
--- a/javascript/ql/src/Security/CWE-116/IncompleteSanitization.ql
+++ b/javascript/ql/src/Security/CWE-116/IncompleteSanitization.ql
@@ -20,36 +20,31 @@ import javascript
 string metachar() { result = "'\"\\&<>\n\r\t*|{}[]%$".charAt(_) }

 /** Gets a string matched by `e` in a `replace` call. */
-string getAMatchedString(Expr e) {
-  result = getAMatchedConstant(e.(RegExpLiteral).getRoot()).getValue()
+string getAMatchedString(DataFlow::Node e) {
+  result = e.(DataFlow::RegExpLiteralNode).getRoot().getAMatchedString()
  or
  result = e.getStringValue()
 }

-/** Gets a constant matched by `t`. */
-RegExpConstant getAMatchedConstant(RegExpTerm t) {
-  result = t
-  or
-  result = getAMatchedConstant(t.(RegExpAlt).getAlternative())
-  or
-  result = getAMatchedConstant(t.(RegExpGroup).getAChild())
-  or
-  exists(RegExpCharacterClass recc | recc = t and not recc.isInverted() |
-    result = getAMatchedConstant(recc.getAChild())
-  )
-}
-
 /** Holds if `t` is simple, that is, a union of constants. */
 predicate isSimple(RegExpTerm t) {
  t instanceof RegExpConstant
  or
  isSimple(t.(RegExpGroup).getAChild())
  or
-  (
-    t instanceof RegExpAlt
-    or
-    t instanceof RegExpCharacterClass and not t.(RegExpCharacterClass).isInverted()
-  ) and
+  isSimpleCharacterClass(t)
+  or
+  isSimpleAlt(t)
+}
+
+/** Holds if `t` is a non-inverted character class that contains no ranges. */
+predicate isSimpleCharacterClass(RegExpCharacterClass t) {
+  not t.isInverted() and
+  forall(RegExpTerm ch | ch = t.getAChild() | isSimple(ch))
+}
+
+/** Holds if `t` is an alternation of simple terms. */
+predicate isSimpleAlt(RegExpAlt t) {
  forall(RegExpTerm ch | ch = t.getAChild() | isSimple(ch))
 }

@@ -57,16 +52,15 @@ predicate isSimple(RegExpTerm t) {
 * Holds if `mce` is of the form `x.replace(re, new)`, where `re` is a global
 * regular expression and `new` prefixes the matched string with a backslash.
 */
-predicate isBackslashEscape(MethodCallExpr mce, RegExpLiteral re) {
-  mce.getMethodName() = "replace" and
-  re.flow().(DataFlow::SourceNode).flowsToExpr(mce.getArgument(0)) and
-  re.isGlobal() and
-  exists(string new | new = mce.getArgument(1).getStringValue() |
-    // `new` is `\$&`, `\$1` or similar
-    new.regexpMatch("\\\\\\$(&|\\d)")
+predicate isBackslashEscape(StringReplaceCall mce, DataFlow::RegExpLiteralNode re) {
+  mce.isGlobal() and
+  re = mce.getRegExp() and
+  (
+    // replacement with `\$&`, `\$1` or similar
+    mce.getRawReplacement().getStringValue().regexpMatch("\\\\\\$(&|\\d)")
    or
-    // `new` is `\c`, where `c` is a constant matched by `re`
-    new.regexpMatch("\\\\\\Q" + getAMatchedString(re) + "\\E")
+    // replacement of `c` with `\c`
+    exists(string c | mce.replaces(c, "\\" + c))
  )
 }

@@ -78,7 +72,7 @@ predicate allBackslashesEscaped(DataFlow::Node nd) {
  nd = DataFlow::globalVarRef("JSON").getAMemberCall("stringify")
  or
  // check whether `nd` itself escapes backslashes
-  exists(RegExpLiteral rel | isBackslashEscape(nd.asExpr(), rel) |
+  exists(DataFlow::RegExpLiteralNode rel | isBackslashEscape(nd, rel) |
    // if it's a complex regexp, we conservatively assume that it probably escapes backslashes
    not isSimple(rel.getRoot()) or
    getAMatchedString(rel) = "\\"
@@ -104,10 +98,8 @@ predicate allBackslashesEscaped(DataFlow::Node nd) {
 /**
 * Holds if `repl` looks like a call to "String.prototype.replace" that deliberately removes the first occurrence of `str`.
 */
-predicate removesFirstOccurence(DataFlow::MethodCallNode repl, string str) {
-  repl.getMethodName() = "replace" and
-  repl.getArgument(0).getStringValue() = str and
-  repl.getArgument(1).getStringValue() = ""
+predicate removesFirstOccurence(StringReplaceCall repl, string str) {
+  not exists(repl.getRegExp()) and repl.replaces(str, "")
 }

 /**
@@ -134,25 +126,30 @@ predicate isDelimiterUnwrapper(
 }

 /*
- * Holds if `repl` is a standalone use of `String.prototype.replace` to remove a single newline.
+ * Holds if `repl` is a standalone use of `String.prototype.replace` to remove a single newline,
+ * dollar or percent character.
+ *
+ * This is often done on inputs that are known to only contain a single instance of the character,
+ * such as output from a shell command that is known to end with a single newline, or strings
+ * like "$1.20" or "50%".
 */

-predicate removesTrailingNewLine(DataFlow::MethodCallNode repl) {
-  repl.getMethodName() = "replace" and
-  repl.getArgument(0).mayHaveStringValue("\n") and
-  repl.getArgument(1).mayHaveStringValue("") and
-  not exists(DataFlow::MethodCallNode other | other.getMethodName() = "replace" |
-    repl.getAMethodCall() = other or
-    other.getAMethodCall() = repl
+predicate whitelistedRemoval(StringReplaceCall repl) {
+  not repl.isGlobal() and
+  exists(string s | s = "\n" or s = "%" or s = "$" |
+    repl.replaces(s, "") and
+    not exists(StringReplaceCall other |
+      repl.getAMethodCall() = other or
+      other.getAMethodCall() = repl
+    )
  )
 }

-from MethodCallExpr repl, Expr old, string msg
+from StringReplaceCall repl, DataFlow::Node old, string msg
 where
-  repl.getMethodName() = "replace" and
-  (old = repl.getArgument(0) or old.flow().(DataFlow::SourceNode).flowsToExpr(repl.getArgument(0))) and
+  (old = repl.getArgument(0) or old = repl.getRegExp()) and
  (
-    not old.(RegExpLiteral).isGlobal() and
+    not repl.isGlobal() and
    msg = "This replaces only the first occurrence of " + old + "." and
    // only flag if this is likely to be a sanitizer or URL encoder or decoder
    exists(string m | m = getAMatchedString(old) |
@@ -166,19 +163,22 @@ where
        // URL encoder
        repl.getArgument(1).getStringValue().regexpMatch(urlEscapePattern)
      )
+      or
+      // path sanitizer
+      (m = ".." or m = "/.." or m = "../" or m = "/../")
    ) and
    // don't flag replace operations in a loop
-    not DataFlow::valueNode(repl.getReceiver()) = DataFlow::valueNode(repl).getASuccessor+() and
+    not repl.getReceiver() = repl.getASuccessor+() and
    // dont' flag unwrapper
-    not isDelimiterUnwrapper(repl.flow(), _) and
-    not isDelimiterUnwrapper(_, repl.flow()) and
-    // dont' flag the removal of trailing newlines
-    not removesTrailingNewLine(repl.flow())
+    not isDelimiterUnwrapper(repl, _) and
+    not isDelimiterUnwrapper(_, repl) and
+    // don't flag replacements of certain characters with whitespace
+    not whitelistedRemoval(repl)
    or
-    exists(RegExpLiteral rel |
+    exists(DataFlow::RegExpLiteralNode rel |
      isBackslashEscape(repl, rel) and
-      not allBackslashesEscaped(DataFlow::valueNode(repl)) and
+      not allBackslashesEscaped(repl) and
      msg = "This does not escape backslash characters in the input."
    )
  )
-select repl.getCallee(), msg
+select repl.getCalleeNode(), msg
--- a/javascript/ql/src/Statements/UseOfReturnlessFunction.ql
+++ b/javascript/ql/src/Statements/UseOfReturnlessFunction.ql
@@ -33,7 +33,9 @@ predicate benignContext(Expr e) {
  inVoidContext(e) or 
  
  // A return statement is often used to just end the function.
-  e = any(Function f).getAReturnedExpr()
+  e = any(Function f).getBody()
+  or
+  e = any(ReturnStmt r).getExpr()
  or 
  exists(ConditionalExpr cond | cond.getABranch() = e and benignContext(cond)) 
  or 
@@ -42,7 +44,6 @@ predicate benignContext(Expr e) {
  exists(Expr parent | parent.getUnderlyingValue() = e and benignContext(parent))
  or 
  any(VoidExpr voidExpr).getOperand() = e
-
  or
  // weeds out calls inside HTML-attributes.
  e.getParent().(ExprStmt).getParent() instanceof CodeInAttribute or
@@ -70,8 +71,8 @@ predicate benignContext(Expr e) {
  e = any(ResolvedPromiseDefinition promise).getValue().asExpr()
 }

-predicate oneshotClosure(InvokeExpr call) {
-  call.getCallee().getUnderlyingValue() instanceof ImmediatelyInvokedFunctionExpr
+predicate oneshotClosure(DataFlow::CallNode call) {
+  call.getCalleeNode().asExpr().getUnderlyingValue() instanceof ImmediatelyInvokedFunctionExpr
 }

 predicate alwaysThrows(Function f) {
@@ -149,6 +150,12 @@ predicate voidArrayCallback(DataFlow::CallNode call, Function func) {
  )
 }

+predicate hasNonVoidReturnType(Function f) {
+  exists(TypeAnnotation type | type = f.getReturnTypeAnnotation() | 
+    not type.isVoid()
+  )
+}
+

 /**
 * Provides classes for working with various Deferred implementations. 
@@ -214,6 +221,8 @@ where
  not benignContext(call.getEnclosingExpr()) and
  not lastStatementHasNoEffect(func) and
  // anonymous one-shot closure. Those are used in weird ways and we ignore them.
-  not oneshotClosure(call.getEnclosingExpr())
+  not oneshotClosure(call) and
+  not hasNonVoidReturnType(func) and
+  not call.getEnclosingExpr() instanceof SuperCall
 select
  call, msg, func, name
--- a/javascript/ql/src/javascript.qll
+++ b/javascript/ql/src/javascript.qll
@@ -76,6 +76,7 @@ import semmle.javascript.frameworks.Electron
 import semmle.javascript.frameworks.Files
 import semmle.javascript.frameworks.Firebase
 import semmle.javascript.frameworks.jQuery
+import semmle.javascript.frameworks.Handlebars
 import semmle.javascript.frameworks.LodashUnderscore
 import semmle.javascript.frameworks.Logging
 import semmle.javascript.frameworks.HttpFrameworks
--- a/javascript/ql/src/semmle/javascript/NodeJS.qll
+++ b/javascript/ql/src/semmle/javascript/NodeJS.qll
@@ -166,7 +166,7 @@ class Require extends CallExpr, Import {
    exists(RequireVariable req |
      this.getCallee() = req.getAnAccess() and
      // `mjs` files explicitly disallow `require`
-      getFile().getExtension() != "mjs"
+      not getFile().getExtension() = "mjs"
    )
  }

--- a/javascript/ql/src/semmle/javascript/Regexp.qll
+++ b/javascript/ql/src/semmle/javascript/Regexp.qll
@@ -173,6 +173,23 @@ class RegExpTerm extends Locatable, @regexpterm {
      parent.(StringLiteral).flow() instanceof RegExpPatternSource
    )
  }
+
+  /**
+   * Gets the single string this regular-expression term matches.
+   *
+   * This predicate is only defined for (sequences/groups of) constant regular expressions.
+   * In particular, terms involving zero-width assertions like `^` or `\b` are not considered
+   * to have a constant value.
+   *
+   * Note that this predicate does not take flags of the enclosing regular-expression literal
+   * into account.
+   */
+  string getConstantValue() { none() }
+
+  /**
+   * Gets a string that is matched by this regular-expression term.
+   */
+  string getAMatchedString() { result = getConstantValue() }
 }

 /**
@@ -223,6 +240,8 @@ class RegExpConstant extends RegExpTerm, @regexp_constant {
  predicate isCharacter() { any() }

  override predicate isNullable() { none() }
+
+  override string getConstantValue() { result = getValue() }
 }

 /**
@@ -266,6 +285,8 @@ class RegExpAlt extends RegExpTerm, @regexp_alt {
  int getNumAlternative() { result = getNumChild() }

  override predicate isNullable() { getAlternative().isNullable() }
+
+  override string getAMatchedString() { result = getAlternative().getAMatchedString() }
 }

 /**
@@ -289,6 +310,21 @@ class RegExpSequence extends RegExpTerm, @regexp_seq {
  override predicate isNullable() {
    forall(RegExpTerm child | child = getAChild() | child.isNullable())
  }
+
+  override string getConstantValue() {
+    result = getConstantValue(0)
+  }
+
+  /**
+   * Gets the single string matched by the `i`th child and all following children of
+   * this sequence, if any.
+   */
+  private string getConstantValue(int i) {
+    i = getNumChild() and
+    result = ""
+    or
+    result = getChild(i).getConstantValue() + getConstantValue(i+1)
+  }
 }

 /**
@@ -549,6 +585,10 @@ class RegExpGroup extends RegExpTerm, @regexp_group {
  string getName() { isNamedCapture(this, result) }

  override predicate isNullable() { getAChild().isNullable() }
+
+  override string getConstantValue() { result = getAChild().getConstantValue() }
+
+  override string getAMatchedString() { result = getAChild().getAMatchedString() }
 }

 /**
@@ -734,6 +774,10 @@ class RegExpCharacterClass extends RegExpTerm, @regexp_char_class {
  predicate isInverted() { isInverted(this) }

  override predicate isNullable() { none() }
+
+  override string getAMatchedString() {
+    not isInverted() and result = getAChild().getAMatchedString()
+  }
 }

 /**
--- a/javascript/ql/src/semmle/javascript/StandardLibrary.qll
+++ b/javascript/ql/src/semmle/javascript/StandardLibrary.qll
@@ -248,3 +248,62 @@ private class IteratorExceptionStep extends DataFlow::MethodCallNode, DataFlow::
    succ = this.getExceptionalReturn()
  }
 }
+
+/**
+ * A call to `String.prototype.replace`.
+ *
+ * We heuristically include any call to a method called `replace`, provided it either
+ * has exactly two arguments, or local data flow suggests that the receiver may be a string.
+ */
+class StringReplaceCall extends DataFlow::MethodCallNode {
+  StringReplaceCall() {
+    getMethodName() = "replace" and
+    (getNumArgument() = 2 or getReceiver().mayHaveStringValue(_))
+  }
+
+  /** Gets the regular expression passed as the first argument to `replace`, if any. */
+  DataFlow::RegExpLiteralNode getRegExp() {
+    result.flowsTo(getArgument(0))
+  }
+
+  /** Gets a string that is being replaced by this call. */
+  string getAReplacedString() {
+    result = getRegExp().getRoot().getAMatchedString() or
+    getArgument(0).mayHaveStringValue(result)
+  }
+
+  /**
+   * Gets the second argument of this call to `replace`, which is either a string
+   * or a callback.
+   */
+  DataFlow::Node getRawReplacement() {
+    result = getArgument(1)
+  }
+
+  /**
+   * Holds if this is a global replacement, that is, the first argument is a regular expression
+   * with the `g` flag.
+   */
+  predicate isGlobal() {
+    getRegExp().isGlobal()
+  }
+
+  /**
+   * Holds if this call to `replace` replaces `old` with `new`.
+   */
+  predicate replaces(string old, string new) {
+    exists(string rawNew |
+      old = getAReplacedString() and
+      getRawReplacement().mayHaveStringValue(rawNew) and
+      new = rawNew.replaceAll("$&", old)
+    )
+    or
+    exists(DataFlow::FunctionNode replacer, DataFlow::PropRead pr, DataFlow::ObjectLiteralNode map |
+      replacer = getCallback(1) and
+      replacer.getParameter(0).flowsToExpr(pr.getPropertyNameExpr()) and
+      pr = map.getAPropertyRead() and
+      pr.flowsTo(replacer.getAReturn()) and
+      map.hasPropertyWrite(old, any(DataFlow::Node repl | repl.getStringValue() = new))
+    )
+  }
+}
--- a/javascript/ql/src/semmle/javascript/TypeScript.qll
+++ b/javascript/ql/src/semmle/javascript/TypeScript.qll
@@ -1816,7 +1816,7 @@ class Type extends @type {
   *
   * For example, for a type `(S & T) | U` this gets the types `S`, `T`, and `U`.
   */
-  Type unfold() {
+  Type unfoldUnionAndIntersection() {
    not result instanceof UnionOrIntersectionType and
    (
      result = this
@@ -1829,6 +1829,27 @@ class Type extends @type {
    )
  }

+  /**
+   * Repeatedly unfolds unions, intersections, and type aliases and gets any of the underlying types,
+   * or this type itself if it is not a union or intersection.
+   *
+   * For example, the type `(S & T) | U` unfolds to `S`, `T`, and `U`.
+   *
+   * If this is a type alias, the alias is itself included in the result, but this is not the case for intermediate type aliases.
+   * For example:
+   * ```js
+   * type One = number | string;
+   * type Two = One | Function & {x: string};
+   * One; // unfolds to number, string, and One
+   * Two; // unfolds to number, string, One, Function, {x: string}, and Two
+   * ```
+   */
+  Type unfold() {
+    result = unfoldUnionAndIntersection()
+    or
+    result = this.(TypeAliasReference).getAliasedType().unfoldUnionAndIntersection()
+  }
+
  /**
   * Holds if this refers to the given named type, or is declared as a subtype thereof,
   * or is a union or intersection containing such a type.
@@ -2287,6 +2308,24 @@ class EnumLiteralType extends TypeReference {
  EnumMember getEnumMember() { result = declaration }
 }

+/**
+ * A type that refers to a type alias.
+ */
+class TypeAliasReference extends TypeReference {
+  TypeAliasReference() {
+    type_alias(this, _)
+  }
+
+  /**
+   * Gets the type behind the type alias.
+   *
+   * For example, for `type B<T> = T[][]`, this maps the type `B<number>` to `number[][]`.
+   */
+  Type getAliasedType() {
+    type_alias(this, result)
+  }
+}
+
 /**
 * An anonymous interface type, such as `{ x: number }`.
 */
@@ -2516,17 +2555,19 @@ class CallSignatureType extends @signature_type {
  predicate hasTypeParameters() { getNumTypeParameter() > 0 }

  /**
-   * Gets the type of the `n`th parameter of this signature.
+   * Gets the type of the `n`th parameter declared in this signature.
+   *
+   * If the `n`th parameter is a rest parameter `...T[]`, gets type `T`.
   */
  Type getParameter(int n) { n >= 0 and result = getChild(n + getNumTypeParameter()) }

  /**
-   * Gets the type of a parameter of this signature.
+   * Gets the type of a parameter of this signature, including the rest parameter, if any.
   */
  Type getAParameter() { result = getParameter(_) }

  /**
-   * Gets the number of parameters.
+   * Gets the number of parameters, including the rest parameter, if any.
   */
  int getNumParameter() { result = count(int i | exists(getParameter(i))) }

@@ -2538,7 +2579,7 @@ class CallSignatureType extends @signature_type {

  /**
   * Gets the number of optional parameters, that is,
-   * parameters that are marked as optional with the `?` suffix.
+   * parameters that are marked as optional with the `?` suffix or is a rest parameter.
   */
  int getNumOptionalParameter() { result = getNumParameter() - getNumRequiredParameter() }

@@ -2552,7 +2593,9 @@ class CallSignatureType extends @signature_type {
  }

  /**
-   * Holds if the `n`th parameter is declared optional with the `?` suffix.
+   * Holds if the `n`th parameter is declared optional with the `?` suffix or is the rest parameter.
+   *
+   * Note that rest parameters are not considered optional in this sense.
   */
  predicate isOptionalParameter(int n) {
    exists(getParameter(n)) and
@@ -2571,6 +2614,30 @@ class CallSignatureType extends @signature_type {
   * Gets the name of a parameter of this signature.
   */
  string getAParameterName() { result = getParameterName(_) }
+
+  /**
+   * Holds if this signature declares a rest parameter, such as `(x: number, ...y: string[])`.
+   */
+  predicate hasRestParameter() { signature_rest_parameter(this, _) }
+
+  /**
+   * Gets the type of the rest parameter, if any.
+   *
+   * For example, for the signature `(...y: string[])`, this gets the type `string`.
+   */
+  Type getRestParameterType() {
+    hasRestParameter() and
+    result = getParameter(getNumParameter() - 1)
+  }
+
+  /**
+   * Gets the type of the rest parameter as an array, if it exists.
+   *
+   * For example, for the signature `(...y: string[])`, this gets the type `string[]`.
+   */
+  PlainArrayType getRestParameterArrayType() {
+    signature_rest_parameter(this, result)
+  }
 }

 /**
--- a/javascript/ql/src/semmle/javascript/Variables.qll
+++ b/javascript/ql/src/semmle/javascript/Variables.qll
@@ -749,6 +749,21 @@ class Parameter extends BindingPattern {
  JSDocTag getJSDocTag() {
    none() // overridden in SimpleParameter
  }
+
+  /**
+   * Holds if this is a parameter declared optional with the `?` token.
+   *
+   * Note that this does not hold for rest parameters, and does not in general
+   * hold for parameters with defaults.
+   *
+   * For example, `x`, is declared optional below:
+   * ```
+   * function f(x?: number) {}
+   * ```
+   */
+  predicate isDeclaredOptional() {
+    isOptionalParameterDeclaration(this)
+  }
 }

 /**
--- a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll
@@ -530,6 +530,24 @@ class ArrayLiteralNode extends DataFlow::ValueNode, DataFlow::SourceNode {
  int getSize() { result = astNode.getSize() }
 }

+/**
+ * A data-flow node corresponding to a regular-expression literal.
+ *
+ * Examples:
+ * ```js
+ * /https?/i
+ * ```
+ */
+class RegExpLiteralNode extends DataFlow::ValueNode, DataFlow::SourceNode {
+  override RegExpLiteral astNode;
+
+  /** Holds if this regular expression has a `g` flag. */
+  predicate isGlobal() { astNode.isGlobal() }
+
+  /** Gets the root term of this regular expression. */
+  RegExpTerm getRoot() { result = astNode.getRoot() }
+}
+
 /**
 * A data flow node corresponding to a `new Array()` or `Array()` invocation.
 *
--- a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll
@@ -676,6 +676,20 @@ module TaintTracking {
    }
  }

+  /**
+   * A taint step through the Node.JS function `util.inspect(..)`.
+   */
+  class UtilInspectTaintStep extends AdditionalTaintStep, DataFlow::InvokeNode {
+    UtilInspectTaintStep() {
+      this = DataFlow::moduleImport("util").getAMemberCall("inspect")
+    }
+
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      succ = this and
+      this.getAnArgument() = pred
+    }
+  }
+
  /**
   * A conditional checking a tainted string against a regular expression, which is
   * considered to be a sanitizer for all configurations.
--- a/javascript/ql/src/semmle/javascript/frameworks/Handlebars.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/Handlebars.qll
@@ -0,0 +1,29 @@
+/**
+ * Provides classes for working with Handlebars code.
+ */
+
+import javascript
+
+module Handlebars {
+  /**
+   * A reference to the Handlebars library. 
+   */
+  class Handlebars extends DataFlow::SourceNode {
+    Handlebars() {
+      this.accessesGlobal("handlebars")
+      or
+      this.accessesGlobal("Handlebars")
+      or
+      this = DataFlow::moduleImport("handlebars")
+      or
+      this.hasUnderlyingType("Handlebars")
+    }
+  }
+
+  /**
+   * A new instantiation of a Handlebars.SafeString.
+   */
+  class SafeString extends DataFlow::NewNode {
+    SafeString() { this = any(Handlebars h).getAConstructorInvocation("SafeString") }
+  }
+}
--- a/javascript/ql/src/semmle/javascript/frameworks/Logging.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/Logging.qll
@@ -61,6 +61,15 @@ private module Console {
      if name = "assert"
      then result = getArgument([1 .. getNumArgument()])
      else result = getAnArgument()
+      or
+      result = getASpreadArgument()
+    }
+
+    /**
+     * Gets the name of the console logging method, e.g. "log", "error", "assert", etc.
+     */
+    string getName() {
+      result = name
    }
  }
 }
--- a/javascript/ql/src/semmle/javascript/frameworks/React.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/React.qll
@@ -136,7 +136,12 @@ abstract class ReactComponent extends ASTNode {
        result = arg0
    )
    or
-    result.flowsToExpr(getStaticMethod("getDerivedStateFromProps").getAReturnedExpr())
+    exists(string staticMember |
+      staticMember = "getDerivedStateFromProps" or
+      staticMember = "getDerivedStateFromError"
+    |
+      result.flowsToExpr(getStaticMethod(staticMember).getAReturnedExpr())
+    )
    or
    // shouldComponentUpdate: (nextProps, nextState)
    result = DataFlow::parameterNode(getInstanceMethod("shouldComponentUpdate").getParameter(1))
--- a/javascript/ql/src/semmle/javascript/security/dataflow/CleartextLogging.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/CleartextLogging.qll
@@ -13,7 +13,7 @@ module CleartextLogging {
  import CleartextLoggingCustomizations::CleartextLogging

  /**
-   * A dataflow tracking configuration for clear-text logging of sensitive information.
+   * A taint tracking configuration for clear-text logging of sensitive information.
   *
   * This configuration identifies flows from `Source`s, which are sources of
   * sensitive data, to `Sink`s, which is an abstract class representing all
@@ -21,27 +21,37 @@ module CleartextLogging {
   * added either by extending the relevant class, or by subclassing this configuration itself,
   * and amending the sources and sinks.
   */
-  class Configuration extends DataFlow::Configuration {
+  class Configuration extends TaintTracking::Configuration {
    Configuration() { this = "CleartextLogging" }

-    override predicate isSource(DataFlow::Node source) { source instanceof Source }
+    override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel lbl) {
+      source.(Source).getLabel() = lbl
+    }

-    override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+    override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel lbl) {
+      sink.(Sink).getLabel() = lbl
+    }

-    override predicate isBarrier(DataFlow::Node node) { node instanceof Barrier }
+    override predicate isSanitizer(DataFlow::Node node) { node instanceof Barrier }

-    override predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) {
-      StringConcatenation::taintStep(src, trg)
-      or
-      exists(string name | name = "toString" or name = "valueOf" |
-        src.(DataFlow::SourceNode).getAMethodCall(name) = trg
-      )
-      or
+    override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
+      succ.(DataFlow::PropRead).getBase() = pred
+    }
+       
+    override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node trg) {
      // A taint propagating data flow edge through objects: a tainted write taints the entire object.
      exists(DataFlow::PropWrite write |
        write.getRhs() = src and
        trg.(DataFlow::SourceNode).flowsTo(write.getBase())
      )
+      or
+      // Taint through the arguments object.
+      exists(DataFlow::CallNode call, Function f |
+        src = call.getAnArgument() and
+        f = call.getACallee() and
+        not call.isImprecise() and
+        trg.asExpr() = f.getArgumentsVariable().getAnAccess()
+      )
    }
  }
 }
--- a/javascript/ql/src/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll
@@ -15,17 +15,39 @@ module CleartextLogging {
  abstract class Source extends DataFlow::Node {
    /** Gets a string that describes the type of this data flow source. */
    abstract string describe();
+    
+    abstract DataFlow::FlowLabel getLabel();
  }

  /**
   * A data flow sink for clear-text logging of sensitive information.
   */
-  abstract class Sink extends DataFlow::Node { }
+  abstract class Sink extends DataFlow::Node {
+    DataFlow::FlowLabel getLabel() {
+      result.isDataOrTaint()
+    }
+  }

  /**
   * A barrier for clear-text logging of sensitive information.
   */
  abstract class Barrier extends DataFlow::Node { }
+  
+  /**
+   * A call to `.replace()` that seems to mask sensitive information.
+   */
+  class MaskingReplacer extends Barrier, DataFlow::MethodCallNode {
+    MaskingReplacer() {
+      this.getCalleeName() = "replace" and
+      exists(RegExpLiteral reg |
+        reg = this.getArgument(0).getALocalSource().asExpr() and
+        reg.isGlobal() and
+        any(RegExpDot term).getLiteral() = reg
+      )
+      and
+      exists(this.getArgument(1).getStringValue())
+    }
+  }

  /**
   * An argument to a logging mechanism.
@@ -107,6 +129,10 @@ module CleartextLogging {
    }

    override string describe() { result = "an access to " + name }
+    
+    override DataFlow::FlowLabel getLabel() {
+      result.isData()
+    }
  }

  /** An access to a variable or property that might contain a password. */
@@ -131,6 +157,10 @@ module CleartextLogging {
    }

    override string describe() { result = "an access to " + name }
+    
+    override DataFlow::FlowLabel getLabel() {
+      result.isData()
+    }
  }

  /** A call that might return a password. */
@@ -143,5 +173,33 @@ module CleartextLogging {
    }

    override string describe() { result = "a call to " + name }
+    
+    override DataFlow::FlowLabel getLabel() {
+      result.isData()
+    }
+  }
+
+  /** An access to the sensitive object `process.env`. */
+  class ProcessEnvSource extends Source {
+    ProcessEnvSource() {
+      this = NodeJSLib::process().getAPropertyRead("env")
+    }
+
+    override string describe() { result = "process environment" }
+    
+    override DataFlow::FlowLabel getLabel() {
+      result.isData() or 
+      result instanceof PartiallySensitiveMap
+    }
+  }
+  
+  /**
+   * A flow label describing a map that might contain sensitive information in some properties.
+   * Property reads on such maps where the property name is fixed is unlikely to leak sensitive information. 
+   */
+  class PartiallySensitiveMap extends DataFlow::FlowLabel {
+    PartiallySensitiveMap() {
+      this = "PartiallySensitiveMap"
+    }
  }
 }
--- a/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll
@@ -95,6 +95,8 @@ module DomBasedXss {
        mcn.getMethodName() = m and
        this = mcn.getArgument(1)
      )
+      or
+      this = any(Handlebars::SafeString s).getAnArgument()
    }
  }

--- a/javascript/ql/src/semmlecode.javascript.dbscheme
+++ b/javascript/ql/src/semmlecode.javascript.dbscheme
@@ -520,6 +520,7 @@ hasProtectedKeyword (int id: @property ref);
 hasReadonlyKeyword (int id: @property ref);
 isOptionalMember (int id: @property ref);
 hasDefiniteAssignmentAssertion (int id: @field_or_vardeclarator ref);
+isOptionalParameterDeclaration (unique int parameter: @pattern ref);

 #keyset[constructor, param_index]
 parameter_fields(
@@ -703,6 +704,10 @@ type_property(
  varchar(900) name: string ref,
  int propertyType: @type ref);

+type_alias(
+  unique int aliasType: @type ref,
+  int underlyingType: @type ref);
+
@literaltype = @stringliteraltype | @numberliteraltype | @booleanliteraltype | @bigintliteraltype;
@type_with_literal_value = @stringliteraltype | @numberliteraltype | @bigintliteraltype;
 type_literal_value(
@@ -717,6 +722,11 @@ signature_types (
  int required_params: int ref
 );

+signature_rest_parameter(
+  unique int sig: @signature_type ref,
+  int rest_param_arra_type: @type ref
+);
+
 case @signature_type.kind of
  0 = @function_signature_type
 | 1 = @constructor_signature_type
--- a/javascript/ql/src/semmlecode.javascript.dbscheme.stats
+++ b/javascript/ql/src/semmlecode.javascript.dbscheme.stats
@@ -8006,6 +8006,17 @@
 <dependencies/>
 </relation>
 <relation>
+<name>isOptionalParameterDeclaration</name>
+<cardinality>3966</cardinality>
+<columnsizes>
+<e>
+<k>parameter</k>
+<v>3966</v>
+</e>
+</columnsizes>
+<dependencies/>
+</relation>
+<relation>
 <name>parameter_fields</name>
 <cardinality>2693</cardinality>
 <columnsizes>
@@ -13516,6 +13527,54 @@
 <dependencies/>
 </relation>
 <relation>
+<name>type_alias</name>
+<cardinality>1386</cardinality>
+<columnsizes>
+<e>
+<k>aliasType</k>
+<v>1386</v>
+</e>
+<e>
+<k>underlyingType</k>
+<v>1361</v>
+</e>
+</columnsizes>
+<dependencies>
+<dep>
+<src>underlyingType</src>
+<trg>aliasType</trg>
+<val>
+<hist>
+<budget>12</budget>
+<bs>
+<b>
+<a>1</a>
+<b>2</b>
+<v>1</v>
+</b>
+</bs>
+</hist>
+</val>
+</dep>
+<dep>
+<src>aliasType</src>
+<trg>underlyingType</trg>
+<val>
+<hist>
+<budget>12</budget>
+<bs>
+<b>
+<a>1</a>
+<b>2</b>
+<v>1</v>
+</b>
+</bs>
+</hist>
+</val>
+</dep>
+</dependencies>
+</relation>
+<relation>
 <name>type_literal_value</name>
 <cardinality>31882</cardinality>
 <columnsizes>
@@ -14222,6 +14281,54 @@
 </dependencies>
 </relation>
 <relation>
+<name>signature_rest_parameter</name>
+<cardinality>19521</cardinality>
+<columnsizes>
+<e>
+<k>sig</k>
+<v>19521</v>
+</e>
+<e>
+<k>rest_param_arra_type</k>
+<v>14259</v>
+</e>
+</columnsizes>
+<dependencies>
+<dep>
+<src>rest_param_arra_type</src>
+<trg>sig</trg>
+<val>
+<hist>
+<budget>12</budget>
+<bs>
+<b>
+<a>1</a>
+<b>2</b>
+<v>1</v>
+</b>
+</bs>
+</hist>
+</val>
+</dep>
+<dep>
+<src>sig</src>
+<trg>rest_param_arra_type</trg>
+<val>
+<hist>
+<budget>12</budget>
+<bs>
+<b>
+<a>1</a>
+<b>2</b>
+<v>1</v>
+</b>
+</bs>
+</hist>
+</val>
+</dep>
+</dependencies>
+</relation>
+<relation>
 <name>type_contains_signature</name>
 <cardinality>87640</cardinality>
 <columnsizes>