JS: use original sanitizer for SSRF query

Author: asger-semmle

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

@@ -58,7 +58,7 @@ module ClientSideUrlRedirect {
     }
 
     override predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink) {
-      sanitizingPrefixEdge(source, sink)
+      hostnameSanitizingPrefixEdge(source, sink)
     }
 
     override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel f, DataFlow::FlowLabel g) {

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

@@ -43,7 +43,7 @@ module ServerSideUrlRedirect {
     }
 
     override predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink) {
-      sanitizingPrefixEdge(source, sink)
+      hostnameSanitizingPrefixEdge(source, sink)
     }
 
     override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {

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

@@ -6,6 +6,34 @@
 
 import javascript
 
+/**
+ * Holds if the string value of `nd` prevents anything appended after it
+ * from affecting the hostname or path of a URL.
+ *
+ * Specifically, this holds if the string contains `?` or `#`.
+ */
+private predicate hasSanitizingSubstring(DataFlow::Node nd) {
+  nd.asExpr().getStringValue().regexpMatch(".*[?#].*")
+  or
+  hasSanitizingSubstring(StringConcatenation::getAnOperand(nd))
+  or
+  hasSanitizingSubstring(nd.getAPredecessor())
+  or
+  nd.isIncomplete(_)
+}
+
+/**
+ * Holds if data that flows from `source` to `sink` cannot affect the
+ * path or earlier part of the resulting string when interpreted as a URL.
+ *
+ * This is considered as a sanitizing edge for the URL redirection queries.
+ */
+predicate sanitizingPrefixEdge(DataFlow::Node source, DataFlow::Node sink) {
+  exists (DataFlow::Node operator, int n |
+    StringConcatenation::taintStep(source, sink, operator, n) and
+    hasSanitizingSubstring(StringConcatenation::getOperand(operator, [0..n-1])))
+}
+
 /**
  * Holds if the string value of `nd` prevents anything appended after it
  * from affecting the hostname of a URL.
@@ -18,24 +46,24 @@ import javascript
  * In the latter case, the additional prefix check is necessary to avoid a `/` that could be interpreted as
  * the `//` separating the (optional) scheme from the hostname.
  */
-predicate hasSanitizingSubstring(DataFlow::Node nd) {
+private predicate hasHostnameSanitizingSubstring(DataFlow::Node nd) {
   nd.asExpr().getStringValue().regexpMatch(".*([?#]|[^?#:/\\\\][/\\\\]).*") 
   or
-  hasSanitizingSubstring(StringConcatenation::getAnOperand(nd))
+  hasHostnameSanitizingSubstring(StringConcatenation::getAnOperand(nd))
   or
-  hasSanitizingSubstring(nd.getAPredecessor())
+  hasHostnameSanitizingSubstring(nd.getAPredecessor())
   or
   nd.isIncomplete(_)
 }
 
 /**
  * Holds if data that flows from `source` to `sink` cannot affect the
- * hostname of the resulting string when interpreted as a URL.
+ * hostname or scheme of the resulting string when interpreted as a URL.
  *
  * This is considered as a sanitizing edge for the URL redirection queries.
  */
-predicate sanitizingPrefixEdge(DataFlow::Node source, DataFlow::Node sink) {
+predicate hostnameSanitizingPrefixEdge(DataFlow::Node source, DataFlow::Node sink) {
   exists (DataFlow::Node operator, int n |
     StringConcatenation::taintStep(source, sink, operator, n) and
     hasSanitizingSubstring(StringConcatenation::getOperand(operator, [0..n-1])))
-}
+}