JS: Only apply exception propagator when no other summary applies

Previously a few Promise-related methods were special-cased, which is no longer needed.
This commit is contained in:
Asger F
2024-11-21 11:00:49 +01:00
parent 84820adf3c
commit 4e62a512c5
3 changed files with 29 additions and 6 deletions

View File

@@ -438,6 +438,19 @@ abstract class LibraryCallable extends string {
DataFlow::InvokeNode getACallSimple() { none() }
}
/** Internal subclass of `LibraryCallable`, whose member predicates should not be visible on `SummarizedCallable`. */
abstract class LibraryCallableInternal extends LibraryCallable {
bindingset[this]
LibraryCallableInternal() { any() }
/**
* Gets a call to this library callable.
*
* Same as `getACall()` but is evaluated later and may depend negatively on `getACall()`.
*/
DataFlow::InvokeNode getACallStage2() { none() }
}
private predicate isParameterNodeImpl(Node p, DataFlowCallable c, ParameterPosition pos) {
exists(Parameter parameter |
parameter = c.asSourceCallable().(Function).getParameter(pos.asPositional()) and
@@ -1014,7 +1027,11 @@ DataFlowCallable viableCallable(DataFlowCall node) {
or
exists(LibraryCallable callable |
result = MkLibraryCallable(callable) and
node.asOrdinaryCall() = [callable.getACall(), callable.getACallSimple()]
node.asOrdinaryCall() =
[
callable.getACall(), callable.getACallSimple(),
callable.(LibraryCallableInternal).getACallStage2()
]
)
or
result.asSourceCallableNotExterns() = node.asImpliedLambdaCall()

View File

@@ -9,6 +9,7 @@ private import sharedlib.DataFlowImplCommon
private import sharedlib.FlowSummaryImpl::Private as Private
private import sharedlib.FlowSummaryImpl::Public
private import codeql.dataflow.internal.AccessPathSyntax as AccessPathSyntax
private import semmle.javascript.internal.flow_summaries.ExceptionFlow
/**
* A class of callables that are candidates for flow summary modeling.
@@ -131,7 +132,11 @@ ReturnKind getStandardReturnValueKind() { result = MkNormalReturnKind() and Stag
private module FlowSummaryStepInput implements Private::StepsInputSig {
DataFlowCall getACall(SummarizedCallable sc) {
exists(LibraryCallable callable | callable = sc |
result.asOrdinaryCall() = [callable.getACall(), callable.getACallSimple()]
result.asOrdinaryCall() =
[
callable.getACall(), callable.getACallSimple(),
callable.(LibraryCallableInternal).getACallStage2()
]
)
}
}

View File

@@ -5,6 +5,7 @@
private import javascript
private import FlowSummaryUtil
private import semmle.javascript.dataflow.internal.AdditionalFlowInternal
private import semmle.javascript.dataflow.internal.DataFlowPrivate
private import semmle.javascript.dataflow.FlowSummary
private import semmle.javascript.internal.flow_summaries.Promises
@@ -22,14 +23,14 @@ private predicate isCallback(DataFlow::SourceNode node) {
/**
* Summary that propagates exceptions out of callbacks back to the caller.
*/
private class ExceptionFlowSummary extends SummarizedCallable {
private class ExceptionFlowSummary extends SummarizedCallable, LibraryCallableInternal {
ExceptionFlowSummary() { this = "Exception propagator" }
override DataFlow::CallNode getACall() {
override DataFlow::CallNode getACallStage2() {
not exists(result.getACallee()) and
not exists(SummarizedCallable c | result = [c.getACall(), c.getACallSimple()]) and
// Avoid a few common cases where the exception should not propagate back
not result.getCalleeName() =
["then", "catch", "finally", "addEventListener", EventEmitter::on()] and
not result.getCalleeName() = ["addEventListener", EventEmitter::on()] and
not result = promiseConstructorRef().getAnInvocation() and
// Restrict to cases where a callback is known to flow in, as lambda flow in DataFlowImplCommon blows up otherwise
isCallback(result.getAnArgument().getALocalSource())