diff --git a/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/StandardEndpointFilters.qll b/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/StandardEndpointFilters.qll index 6fe866b2651..80033871eed 100644 --- a/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/StandardEndpointFilters.qll +++ b/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/StandardEndpointFilters.qll @@ -17,24 +17,25 @@ predicate isIntermediaryDataflowNode(DataFlow::Node n) { /** Provides a set of reasons why a given data flow node should be excluded as a sink candidate. */ string getAReasonSinkExcluded(DataFlow::Node n) { - isIntermediaryDataflowNode(n) and result = "intermediary dataflow node" - or - 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" + ) ) } @@ -131,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 diff --git a/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/TaintedPathATM.qll b/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/TaintedPathATM.qll index 109fd357007..ae5871c229f 100644 --- a/javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/TaintedPathATM.qll +++ b/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" + ) } }