JS: Preverse tainted-url-suffix when stepping into prefix

A URL of form https://example.com?evil#bar will contain '?evil' after splitting out the '#' suffix, and vice versa.

Author: asgerf

javascript/ql/lib/semmle/javascript/internal/flow_summaries/Strings.qll

@@ -88,10 +88,13 @@ class StringSplitHashOrQuestionMark extends SummarizedCallable {
   override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
     preservesValue = false and
     (
-      input = "Argument[this].OptionalBarrier[tainted-url-suffix]" and
+      input = "Argument[this].OptionalBarrier[split-url-suffix]" and
       output = "ReturnValue.ArrayElement"
       or
-      input = "Argument[this].OptionalStep[tainted-url-suffix]" and
+      input = "Argument[this].OptionalStep[split-url-suffix-pre]" and
+      output = "ReturnValue.ArrayElement[0]"
+      or
+      input = "Argument[this].OptionalStep[split-url-suffix-post]" and
       output = "ReturnValue.ArrayElement[1]" // TODO: support ArrayElement[1..]
     )
   }

javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffix.qll

@@ -44,7 +44,7 @@ module TaintedUrlSuffix {
    */
   predicate isBarrier(Node node, FlowLabel label) {
     label = label() and
-    DataFlowPrivate::optionalBarrier(node, "tainted-url-suffix")
+    DataFlowPrivate::optionalBarrier(node, "split-url-suffix")
   }
 
   /**
@@ -55,7 +55,11 @@ module TaintedUrlSuffix {
   predicate step(Node src, Node dst, FlowLabel srclbl, FlowLabel dstlbl) {
     srclbl = label() and
     dstlbl.isTaint() and
-    DataFlowPrivate::optionalStep(src, "tainted-url-suffix", dst)
+    DataFlowPrivate::optionalStep(src, "split-url-suffix-post", dst)
+    or
+    srclbl = label() and
+    dstlbl = label() and
+    DataFlowPrivate::optionalStep(src, "split-url-suffix-pre", dst)
     or
     // Transition from URL suffix to full taint when extracting the query/fragment part.
     srclbl = label() and

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

@@ -5,14 +5,14 @@ function t1() {
 
     sink(href); // $ flow=tainted-url-suffix
 
-    sink(href.split('#')[0]); // $ MISSING: flow=tainted-url-suffix
+    sink(href.split('#')[0]); // $ flow=tainted-url-suffix
     sink(href.split('#')[1]); // $ flow=taint
-    sink(href.split('#').pop()); // $ flow=taint
+    sink(href.split('#').pop()); // $ flow=taint flow=tainted-url-suffix
     sink(href.split('#')[2]); // $ MISSING: flow=taint // currently the split() summary only propagates to index 1
 
-    sink(href.split('?')[0]); // $ MISSING: flow=tainted-url-suffix
+    sink(href.split('?')[0]); // $ flow=tainted-url-suffix
     sink(href.split('?')[1]); // $ flow=taint
-    sink(href.split('?').pop()); // $ flow=taint
+    sink(href.split('?').pop()); // $ flow=taint flow=tainted-url-suffix
     sink(href.split('?')[2]); // $ MISSING: flow=taint
 
     sink(href.split(blah())[0]); // $ flow=tainted-url-suffix