don't filter sinks due to intermediary notes

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/StandardEndpointFilters.qll

@@ -11,24 +11,31 @@ private import semmle.javascript.filters.ClassifyFiles as ClassifyFiles
 private import semmle.javascript.heuristics.SyntacticHeuristics
 private import CoreKnowledge as CoreKnowledge
 
+predicate isIntermediaryDataflowNode(DataFlow::Node n) {
+  n instanceof DataFlow::ExceptionalInvocationReturnNode
+}
+
 /** Provides a set of reasons why a given data flow node should be excluded as a sink candidate. */
 string getAReasonSinkExcluded(DataFlow::Node n) {
-  isArgumentToModeledFunction(n) and result = "argument to modeled function"
-  or
-  isArgumentToSinklessLibrary(n) and result = "argument to sinkless library"
-  or
-  isSanitizer(n) and result = "sanitizer"
-  or
-  isPredicate(n) and result = "predicate"
-  or
-  isHash(n) and result = "hash"
-  or
-  isNumeric(n) and result = "numeric"
-  or
-  // Ignore candidate sinks within externs, generated, library, and test code
-  exists(string category | category = ["externs", "generated", "library", "test"] |
-    ClassifyFiles::classify(n.getFile(), category) and
-    result = "in " + category + " file"
+  not isIntermediaryDataflowNode(n) and
+  (
+    isArgumentToModeledFunction(n) and result = "argument to modeled function"
+    or
+    isArgumentToSinklessLibrary(n) and result = "argument to sinkless library"
+    or
+    isSanitizer(n) and result = "sanitizer"
+    or
+    isPredicate(n) and result = "predicate"
+    or
+    isHash(n) and result = "hash"
+    or
+    isNumeric(n) and result = "numeric"
+    or
+    // Ignore candidate sinks within externs, generated, library, and test code
+    exists(string category | category = ["externs", "generated", "library", "test"] |
+      ClassifyFiles::classify(n.getFile(), category) and
+      result = "in " + category + " file"
+    )
   )
 }
 
@@ -125,7 +132,7 @@ private DataFlow::SourceNode getACallback(DataFlow::ParameterNode p, DataFlow::T
  * Get calls for which we do not have the callee (i.e. the definition of the called function). This
  * acts as a heuristic for identifying calls to external library functions.
  */
-private DataFlow::CallNode getACallWithoutCallee() {
+private DataFlow::InvokeNode getACallWithoutCallee() {
   forall(Function callee | callee = result.getACallee() | callee.getTopLevel().isExterns()) and
   not exists(DataFlow::ParameterNode param, DataFlow::FunctionNode callback |
     param.flowsTo(result.getCalleeNode()) and

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/TaintedPathATM.qll

@@ -25,37 +25,40 @@ module SinkEndpointFilter {
    * effective sink.
    */
   string getAReasonSinkExcluded(DataFlow::Node sinkCandidate) {
-    result = StandardEndpointFilters::getAReasonSinkExcluded(sinkCandidate)
-    or
-    // Require path injection sink candidates to be (a) arguments to external library calls
-    // (possibly indirectly), or (b) heuristic sinks.
-    //
-    // Heuristic sinks are mostly copied from the `HeuristicTaintedPathSink` class defined within
-    // `codeql/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll`.
-    // We can't reuse the class because importing that file would cause us to treat these
-    // heuristic sinks as known sinks.
-    not StandardEndpointFilters::flowsToArgumentOfLikelyExternalLibraryCall(sinkCandidate) and
-    not (
-      isAssignedToOrConcatenatedWith(sinkCandidate, "(?i)(file|folder|dir|absolute)")
+    not StandardEndpointFilters::isIntermediaryDataflowNode(sinkCandidate) and
+    (
+      result = StandardEndpointFilters::getAReasonSinkExcluded(sinkCandidate)
       or
-      isArgTo(sinkCandidate, "(?i)(get|read)file")
-      or
-      exists(string pathPattern |
-        // paths with at least two parts, and either a trailing or leading slash
-        pathPattern = "(?i)([a-z0-9_.-]+/){2,}" or
-        pathPattern = "(?i)(/[a-z0-9_.-]+){2,}"
-      |
-        isConcatenatedWithString(sinkCandidate, pathPattern)
-      )
-      or
-      isConcatenatedWithStrings(".*/", sinkCandidate, "/.*")
-      or
-      // In addition to the names from `HeuristicTaintedPathSink` in the
-      // `isAssignedToOrConcatenatedWith` predicate call above, we also allow the noisier "path"
-      // name.
-      isAssignedToOrConcatenatedWith(sinkCandidate, "(?i)path")
-    ) and
-    result = "not a direct argument to a likely external library call or a heuristic sink"
+      // Require path injection sink candidates to be (a) arguments to external library calls
+      // (possibly indirectly), or (b) heuristic sinks.
+      //
+      // Heuristic sinks are mostly copied from the `HeuristicTaintedPathSink` class defined within
+      // `codeql/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll`.
+      // We can't reuse the class because importing that file would cause us to treat these
+      // heuristic sinks as known sinks.
+      not StandardEndpointFilters::flowsToArgumentOfLikelyExternalLibraryCall(sinkCandidate) and
+      not (
+        isAssignedToOrConcatenatedWith(sinkCandidate, "(?i)(file|folder|dir|absolute)")
+        or
+        isArgTo(sinkCandidate, "(?i)(get|read)file")
+        or
+        exists(string pathPattern |
+          // paths with at least two parts, and either a trailing or leading slash
+          pathPattern = "(?i)([a-z0-9_.-]+/){2,}" or
+          pathPattern = "(?i)(/[a-z0-9_.-]+){2,}"
+        |
+          isConcatenatedWithString(sinkCandidate, pathPattern)
+        )
+        or
+        isConcatenatedWithStrings(".*/", sinkCandidate, "/.*")
+        or
+        // In addition to the names from `HeuristicTaintedPathSink` in the
+        // `isAssignedToOrConcatenatedWith` predicate call above, we also allow the noisier "path"
+        // name.
+        isAssignedToOrConcatenatedWith(sinkCandidate, "(?i)path")
+      ) and
+      result = "not a direct argument to a likely external library call or a heuristic sink"
+    )
   }
 }