JS: Update syntactic heuristics

javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll

@@ -34,11 +34,11 @@ private class HeuristicCodeInjectionSink extends HeuristicSink, CodeInjection::S
       srcPattern = "(?s).*function\\s*\\(.*\\).*" or
       srcPattern = "(?s).*(\\(.*\\)|[A-Za-z_]+)\\s?=>.*"
     |
-      isContatenatedWithString(this, srcPattern)
+      isConcatenatedWithString(this, srcPattern)
     )
     or
     // dynamic property name
-    isContatenatedWithStrings("(?is)[a-z]+\\[", this, "(?s)\\].*")
+    isConcatenatedWithStrings("(?is)[a-z]+\\[", this, "(?s)\\].*")
   }
 }
 
@@ -53,8 +53,8 @@ private class HeuristicDomBasedXssSink extends HeuristicSink, DomBasedXss::DomBa
   HeuristicDomBasedXssSink() {
     isAssignedToOrConcatenatedWith(this, "(?i)(html|innerhtml)") or
     isArgTo(this, "(?i)(html|render)") or
-    isContatenatedWithString(this, "(?is).*<.*>.*") or
-    isContatenatedWithStrings("(?is).*<[a-z ]+.*", this, "(?s).*>.*")
+    this instanceof StringOps::HtmlConcatenationLeaf or
+    isConcatenatedWithStrings("(?is).*<[a-z ]+.*", this, "(?s).*>.*")
   }
 }
 
@@ -62,8 +62,8 @@ private class HeuristicReflectedXssSink extends HeuristicSink, ReflectedXss::Ref
   HeuristicReflectedXssSink() {
     isAssignedToOrConcatenatedWith(this, "(?i)(html|innerhtml)") or
     isArgTo(this, "(?i)(html|render)") or
-    isContatenatedWithString(this, "(?is).*<.*>.*") or
-    isContatenatedWithStrings("(?is).*<[a-z ]+.*", this, "(?s).*>.*")
+    this instanceof StringOps::HtmlConcatenationLeaf or
+    isConcatenatedWithStrings("(?is).*<[a-z ]+.*", this, "(?s).*>.*")
   }
 }
 
@@ -71,7 +71,7 @@ private class HeuristicSqlInjectionSink extends HeuristicSink, SqlInjection::Sin
   HeuristicSqlInjectionSink() {
     isAssignedToOrConcatenatedWith(this, "(?i)(sql|query)") or
     isArgTo(this, "(?i)(query)") or
-    isContatenatedWithString(this,
+    isConcatenatedWithString(this,
       "(?s).*(ALTER|COUNT|CREATE|DATABASE|DELETE|DISTINCT|DROP|FROM|GROUP|INSERT|INTO|LIMIT|ORDER|SELECT|TABLE|UPDATE|WHERE).*")
   }
 }
@@ -94,10 +94,10 @@ private class HeuristicTaintedPathSink extends HeuristicSink, TaintedPath::Sink
       pathPattern = "(?i)([a-z0-9_.-]+/){2,}" or
       pathPattern = "(?i)(/[a-z0-9_.-]+){2,}"
     |
-      isContatenatedWithString(this, pathPattern)
+      isConcatenatedWithString(this, pathPattern)
     )
     or
-    isContatenatedWithStrings(".*/", this, "/.*")
+    isConcatenatedWithStrings(".*/", this, "/.*")
   }
 }
 

javascript/ql/src/semmle/javascript/heuristics/SyntacticHeuristics.qll

@@ -66,42 +66,28 @@ predicate isArgTo(DataFlow::Node arg, string regexp) {
 }
 
 /**
- * Holds if `n` is concatenated with something with a name that matches `regexp`.
+ * Holds if `n` is concatenation containing something with a name that matches `regexp`.
  */
 bindingset[regexp]
-predicate isConcatenatedWith(DataFlow::Node n, string regexp) {
-  exists(Expr other |
-    other = n.asExpr().(AddExpr).getAnOperand() or
-    other = n.asExpr().(AssignAddExpr).getRhs()
-  |
-    isReadFrom(DataFlow::valueNode(other), regexp)
-  )
+predicate isConcatenatedWith(StringOps::Concatenation n, string regexp) {
+  isReadFrom(n.getAnOperand(), regexp)
 }
 
 /**
- * Holds if `n` is concatenated with a string constant that matches `regexp`.
+ * Holds if `n` is a concatenation containing something with a name that matches `regexp`.
  */
 bindingset[regexp]
-predicate isContatenatedWithString(DataFlow::Node n, string regexp) {
-  exists(Expr other |
-    other = n.asExpr().(AddExpr).getAnOperand() or
-    other = n.asExpr().(AssignAddExpr).getRhs()
-  |
-    other.getStringValue().regexpMatch(regexp)
-  )
+predicate isConcatenatedWithString(StringOps::Concatenation n, string regexp) {
+  n.getAnOperand().getStringValue().regexpMatch(regexp)
 }
 
 /**
  * Holds if `n` is concatenated between two string constants that match `lRegexp` and `rRegexp` respectively.
  */
 bindingset[lRegexp, rRegexp]
-predicate isContatenatedWithStrings(string lRegexp, DataFlow::Node n, string rRegexp) {
-  exists(AddExpr concat1, AddExpr concat2 |
-    concat1.getLeftOperand().getStringValue().regexpMatch(lRegexp) and
-    concat1.getRightOperand() = n.asExpr() and
-    concat2.getLeftOperand() = concat1 and
-    concat2.getRightOperand().getStringValue().regexpMatch(rRegexp)
-  )
+predicate isConcatenatedWithStrings(string lRegexp, StringOps::ConcatenationLeaf n, string rRegexp) {
+  n.getPreviousLeaf().getStringValue().regexpMatch(lRegexp) and
+  n.getNextLeaf().getStringValue().regexpMatch(rRegexp)
 }
 
 /**

javascript/ql/test/library-tests/Security/heuristics/HeuristicSink.expected

@@ -3,6 +3,7 @@
 | sinks.js:4:9:4:12 | sink |
 | sinks.js:6:16:6:19 | sink |
 | sinks.js:7:5:7:22 | getScript() + sink |
+| sinks.js:8:5:8:18 | script += sink |
 | sinks.js:8:15:8:18 | sink |
 | sinks.js:9:5:9:18 | sink += script |
 | sinks.js:10:11:10:14 | sink |
@@ -25,8 +26,11 @@
 | sinks.js:41:5:41:26 | sink +  ... ion(){" |
 | sinks.js:42:5:42:18 | "x => " + sink |
 | sinks.js:43:14:43:17 | sink |
-| sinks.js:45:5:45:18 | "<div>" + sink |
+| sinks.js:45:5:45:11 | "<div>" |
+| sinks.js:45:15:45:18 | sink |
+| sinks.js:46:5:46:20 | '<div foo="foo"' |
 | sinks.js:46:24:46:27 | sink |
+| sinks.js:46:31:46:42 | 'bar="bar">' |
 | sinks.js:48:5:48:20 | "SELECT " + sink |
 | sinks.js:50:5:50:21 | "/foo/bar" + sink |
 | sinks.js:51:5:51:21 | "foo/bar/" + sink |