JavaScript: add string concatenation library

Author: asger-semmle

javascript/ql/src/javascript.qll

@@ -38,6 +38,7 @@ import semmle.javascript.Regexp
 import semmle.javascript.SSA
 import semmle.javascript.StandardLibrary
 import semmle.javascript.Stmt
+import semmle.javascript.StringConcatenation
 import semmle.javascript.Templates
 import semmle.javascript.Tokens
 import semmle.javascript.TypeScript

javascript/ql/src/semmle/javascript/StringConcatenation.qll

@@ -0,0 +1,71 @@
+/**
+ * Provides predicates for analyzing string concatenations and their operands.
+ */
+import javascript
+
+module StringConcatenation {
+  /** Gets a data flow node referring to the result of the given concatenation. */
+  private DataFlow::Node getAssignAddResult(AssignAddExpr expr) {
+    result = expr.flow()
+    or
+    exists (SsaExplicitDefinition def | def.getDef() = expr |
+      result = DataFlow::valueNode(def.getVariable().getAUse()))
+  }
+
+  /** Gets the `n`th operand to the string concatenation defining `node`. */
+  DataFlow::Node getOperand(DataFlow::Node node, int n) {
+    exists (AddExpr add | node = add.flow() |
+      n = 0 and result = add.getLeftOperand().flow()
+      or
+      n = 1 and result = add.getRightOperand().flow())
+    or
+    exists (TemplateLiteral template | node = template.flow() |
+      result = template.getElement(n).flow() and
+      not exists (TaggedTemplateExpr tag | template = tag.getTemplate()))
+    or
+    exists (AssignAddExpr assign | node = getAssignAddResult(assign) |
+      n = 0 and result = assign.getLhs().flow()
+      or
+      n = 1 and result = assign.getRhs().flow())
+    or
+    exists (DataFlow::ArrayCreationNode array |
+      node = array.getAMethodCall("join") and
+      node.(DataFlow::MethodCallNode).getArgument(0).mayHaveStringValue("") and
+      result = array.getElement(n))
+  }
+
+  /** Gets an operand to the string concatenation defining `node`. */
+  DataFlow::Node getAnOperand(DataFlow::Node node) {
+    result = getOperand(node, _)
+  }
+
+  /** Gets the number of operands to the given concatenation. */
+  int getNumOperand(DataFlow::Node node) {
+    result = strictcount(getAnOperand(node))
+  }
+
+  /** Gets the first operand to the string concatenation defining `node`. */
+  DataFlow::Node getFirstOperand(DataFlow::Node node) {
+    result = getOperand(node, 0)
+  }
+
+  /** Gets the last operand to the string concatenation defining `node`. */
+  DataFlow::Node getLastOperand(DataFlow::Node node) {
+    result = getOperand(node, getNumOperand(node) - 1)
+  }
+  
+  /**
+   * Holds if `src` flows to `dst` through the `n`th operand of the given concatenation operator.
+   */
+  predicate taintStep(DataFlow::Node src, DataFlow::Node dst, DataFlow::Node operator, int n) {
+    src = getOperand(dst, n) and
+    operator = dst
+  }
+
+  /**
+   * Holds if there is a taint step from `src` to `dst` through string concatenation.
+   */
+  predicate taintStep(DataFlow::Node src, DataFlow::Node dst) {
+    taintStep(src, dst, _, _)
+  }
+}

javascript/ql/src/semmle/javascript/dataflow/Nodes.qll

@@ -304,6 +304,47 @@ class ArrayLiteralNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode
 
 }
 
+/** A data flow node corresponding to a `new Array()` or `Array()` invocation. */
+class ArrayConstructorInvokeNode extends DataFlow::InvokeNode {
+  ArrayConstructorInvokeNode() {
+    getCallee() = DataFlow::globalVarRef("Array")
+  }
+
+  /** Gets the `i`th initial element of this array, if one is provided. */
+  DataFlow::ValueNode getElement(int i) {
+    getNumArgument() > 1 and // A single-argument invocation specifies the array length, not an element.
+    result = getArgument(i)
+  }
+
+  /** Gets an initial element of this array, if one is provided. */
+  DataFlow::ValueNode getAnElement() {
+    getNumArgument() > 1 and
+    result = getAnArgument()
+  }
+}
+
+/**
+ * A data flow node corresponding to the creation or a new array, either through an array literal
+ * or an invocation of the `Array` constructor.
+ */
+class ArrayCreationNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode {
+  ArrayCreationNode() {
+    this instanceof ArrayLiteralNode or
+    this instanceof ArrayConstructorInvokeNode
+  }
+
+  /** Gets the `i`th initial element of this array, if one is provided. */
+  DataFlow::ValueNode getElement(int i) {
+    result = this.(ArrayLiteralNode).getElement(i) or
+    result = this.(ArrayConstructorInvokeNode).getElement(i)
+  }
+
+  /** Gets an initial element of this array, if one if provided. */
+  DataFlow::ValueNode getAnElement() {
+    result = getElement(_)
+  }
+}
+
 /**
  * A data flow node corresponding to a `default` import from a module, or a
  * (AMD or CommonJS) `require` of a module.

javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll

@@ -358,19 +358,8 @@ module TaintTracking {
    */
   class StringConcatenationTaintStep extends AdditionalTaintStep, DataFlow::ValueNode {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      succ = this and
-      (
-        // addition propagates taint
-        astNode.(AddExpr).getAnOperand() = pred.asExpr() or
-        astNode.(AssignAddExpr).getAChildExpr() = pred.asExpr() or
-        exists (SsaExplicitDefinition ssa |
-          astNode = ssa.getVariable().getAUse() and
-          pred.asExpr().(AssignAddExpr) = ssa.getDef()
-        )
-        or
-        // templating propagates taint
-        astNode.(TemplateLiteral).getAnElement() = pred.asExpr()
-      )
+        succ = this and
+        StringConcatenation::taintStep(pred, succ)
     }
   }
 

javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXss.qll

@@ -77,9 +77,9 @@ module DomBasedXss {
         or
         // or it doesn't start with something other than `<`, and so at least
         // _may_ be interpreted as HTML
-        not exists (Expr prefix, string strval |
+        not exists (DataFlow::Node prefix, string strval |
           isPrefixOfJQueryHtmlString(astNode, prefix) and
-          strval = prefix.getStringValue() and
+          strval = prefix.asExpr().getStringValue() and
           not strval.regexpMatch("\\s*<.*")
         )
       )
@@ -93,13 +93,14 @@ module DomBasedXss {
    * Holds if `prefix` is a prefix of `htmlString`, which may be intepreted as
    * HTML by a jQuery method.
    */
-  private predicate isPrefixOfJQueryHtmlString(Expr htmlString, Expr prefix) {
+  private predicate isPrefixOfJQueryHtmlString(Expr htmlString, DataFlow::Node prefix) {
     any(JQueryMethodCall call).interpretsArgumentAsHtml(htmlString) and
-    prefix = htmlString
+    prefix = htmlString.flow()
     or
-    exists (Expr pred | isPrefixOfJQueryHtmlString(htmlString, pred) |
-      prefix = pred.(AddExpr).getLeftOperand() or
-      prefix = pred.(ParExpr).getExpression()
+    exists (DataFlow::Node pred | isPrefixOfJQueryHtmlString(htmlString, pred) |
+      prefix = StringConcatenation::getFirstOperand(pred)
+      or
+      prefix = pred.getAPredecessor()
     )
   }
 

javascript/ql/src/semmle/javascript/security/dataflow/ServerSideUrlRedirect.qll

@@ -34,33 +34,23 @@ module ServerSideUrlRedirect {
       )
     }
   }
-
-  /**
-   * Gets the left operand of `nd` if it is a concatenation.
-   */
-  private DataFlow::Node getPrefixOperand(DataFlow::Node nd) {
-    exists (Expr e | e instanceof AddExpr or e instanceof AssignAddExpr |
-      nd = DataFlow::valueNode(e) and
-      result = DataFlow::valueNode(e.getChildExpr(0))
-    )
-  }
 
   /**
    * Gets a node that is transitively reachable from `nd` along prefix predecessor edges.
    */
   private DataFlow::Node prefixCandidate(Sink sink) {
     result = sink or
-    result = getPrefixOperand(prefixCandidate(sink)) or
-    result = prefixCandidate(sink).getAPredecessor()
+    result = prefixCandidate(sink).getAPredecessor() or
+    result = StringConcatenation::getFirstOperand(prefixCandidate(sink))
   }
-  
+
   /**
    * Gets an expression that may end up being a prefix of the string concatenation `nd`.
    */
   private Expr getAPrefix(Sink sink) {
     exists (DataFlow::Node prefix |
       prefix = prefixCandidate(sink) and
-      not exists(getPrefixOperand(prefix)) and
+      not exists(StringConcatenation::getFirstOperand(prefix)) and
       not exists(prefix.getAPredecessor()) and
       result = prefix.asExpr()
     )

javascript/ql/src/semmle/javascript/security/dataflow/UrlConcatenation.qll

@@ -11,16 +11,13 @@ import javascript
  * `nd` or one of its operands, assuming that it is a concatenation.
  */
 private predicate hasSanitizingSubstring(DataFlow::Node nd) {
-  exists (Expr e | e = nd.asExpr() |
-    (e instanceof AddExpr or e instanceof AssignAddExpr) and
-    hasSanitizingSubstring(DataFlow::valueNode(e.getAChildExpr()))
-    or
-    e.getStringValue().regexpMatch(".*[?#].*")
-  )
+  nd.asExpr().getStringValue().regexpMatch(".*[?#].*")
   or
-  nd.isIncomplete(_)
+  hasSanitizingSubstring(StringConcatenation::getAnOperand(nd))
   or
   hasSanitizingSubstring(nd.getAPredecessor())
+  or
+  nd.isIncomplete(_)
 }
 
 /**
@@ -30,17 +27,7 @@ private predicate hasSanitizingSubstring(DataFlow::Node nd) {
  * This is considered as a sanitizing edge for the URL redirection queries.
  */
 predicate sanitizingPrefixEdge(DataFlow::Node source, DataFlow::Node sink) {
-  exists (AddExpr add, DataFlow::Node left |
-    source.asExpr() = add.getRightOperand() and
-    sink.asExpr() = add and
-    left.asExpr() = add.getLeftOperand() and
-    hasSanitizingSubstring(left)
-  )
-  or
-  exists (TemplateLiteral tl, int i, DataFlow::Node elt |
-    source.asExpr() = tl.getElement(i) and
-    sink.asExpr() = tl and
-    elt.asExpr() = tl.getElement([0..i-1]) and
-    hasSanitizingSubstring(elt)
-  )
+  exists (DataFlow::Node operator, int n |
+    StringConcatenation::taintStep(source, sink, operator, n) and
+    hasSanitizingSubstring(StringConcatenation::getOperand(operator, [0..n-1])))
 }

javascript/ql/test/library-tests/StringConcatenation/ContainsTwo.expected

@@ -0,0 +1,28 @@
+| tst.js:3:3:3:12 | x += "two" |
+| tst.js:3:8:3:12 | "two" |
+| tst.js:4:3:4:3 | x |
+| tst.js:4:3:4:14 | x += "three" |
+| tst.js:5:3:5:3 | x |
+| tst.js:5:3:5:13 | x += "four" |
+| tst.js:6:10:6:10 | x |
+| tst.js:12:5:12:26 | x += "o ... + "two" |
+| tst.js:12:10:12:26 | "one" + y + "two" |
+| tst.js:12:22:12:26 | "two" |
+| tst.js:19:11:19:23 | "one" + "two" |
+| tst.js:19:19:19:23 | "two" |
+| tst.js:20:3:20:3 | x |
+| tst.js:20:3:20:25 | x += (" ... "four") |
+| tst.js:21:10:21:10 | x |
+| tst.js:21:10:21:19 | x + "five" |
+| tst.js:25:10:25:41 | ["one", ... oin("") |
+| tst.js:25:18:25:22 | "two" |
+| tst.js:29:10:29:46 | Array(" ... oin("") |
+| tst.js:29:23:29:27 | "two" |
+| tst.js:33:10:33:50 | new Arr ... oin("") |
+| tst.js:33:27:33:31 | "two" |
+| tst.js:38:11:38:15 | "two" |
+| tst.js:46:23:46:27 | "two" |
+| tst.js:53:10:53:34 | `one ${ ...  three` |
+| tst.js:53:19:53:23 |  two  |
+| tst.js:71:14:71:18 | "two" |
+| tst.js:77:23:77:27 | "two" |

javascript/ql/test/library-tests/StringConcatenation/ContainsTwo.ql

@@ -0,0 +1,15 @@
+import javascript
+
+// Select all expressions whose string value contains the word "two"
+
+predicate containsTwo(DataFlow::Node node) {
+  node.asExpr().getStringValue().regexpMatch(".*two.*")
+  or
+  containsTwo(node.getAPredecessor())
+  or
+  containsTwo(StringConcatenation::getAnOperand(node))
+}
+
+from Expr e
+where containsTwo(e.flow())
+select e

javascript/ql/test/library-tests/StringConcatenation/tst.js

@@ -0,0 +1,82 @@
+function append() {
+  let x = "one";
+  x += "two";
+  x += "three"
+  x += "four"
+  return x;
+}
+
+function appendClosure(ys) {
+  let x = "first";
+  ys.forEach(y => {
+    x += "one" + y + "two";
+  });
+  x += "last";
+  return x;
+}
+ 
+function appendMixed() {
+  let x = "one" + "two";
+  x += ("three" + "four");
+  return x + "five";
+}
+
+function joinArrayLiteral() {
+  return ["one", "two", "three"].join("");
+}
+
+function joinArrayCall() {
+  return Array("one", "two", "three").join("");
+}
+
+function joinArrayNewCall() {
+  return new Array("one", "two", "three").join("");
+}
+
+function push() {
+  let xs = ["one"];
+  xs.push("two");
+  xs.push("three", "four");
+  return xs.join("");
+}
+
+function pushClosure(ys) {
+  let xs = ["first"];
+  ys.forEach(y => {
+    xs.push("one", y, "two");
+  });
+  xs.push("last");
+  return xs.join("");
+}
+
+function template(x) {
+  return `one ${x} two ${x} three`;
+}
+
+function taggedTemplate(mid) {
+  return someTag`first ${mid} last`;
+}
+
+function templateRepeated(x) {
+  return `first ${x}${x}${x} last`;
+}
+
+function makeArray() {
+  return [];
+}
+
+function pushNoLocalCreation() {
+  let array = makeArray();
+  array.push("one");
+  array.push("two");
+  array.push("three");
+  return array.join("");
+}
+
+function joinInClosure() {
+  let array = ["one", "two", "three"];
+  function f() {
+    return array.join();
+  }
+  return f();
+}