Skip to content

Commit c5a7c00

Browse files
committed
Data flow: Prevent context-sensitive dispatch in source call contexts
1 parent bac28b9 commit c5a7c00

File tree

3 files changed

+30
-14
lines changed

3 files changed

+30
-14
lines changed
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,2 @@
11
unexpectedModel
2-
| Unexpected contentbased-summary found: Models;HigherOrderParameters;false;Apply;(System.Func<System.Object,System.Object>,System.Object);;Argument[1];ReturnValue;value;dfc-generated |
32
expectedModel

shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
413413

414414
Cc ccNone();
415415

416-
CcCall ccSomeCall();
416+
CcCall ccSomeCall(boolean isSource);
417417

418418
/*
419419
* The following `instanceof` predicates are necessary for proper
@@ -566,7 +566,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
566566
Nd node, Cc cc, SummaryCtx summaryCtx, Typ t, Ap ap, ApApprox apa, TypOption stored
567567
) {
568568
sourceNode(node) and
569-
(if hasSourceCallCtx() then cc = ccSomeCall() else cc = ccNone()) and
569+
(if hasSourceCallCtx() then cc = ccSomeCall(true) else cc = ccNone()) and
570570
summaryCtx.isSourceCtx() and
571571
t = getNodeTyp(node) and
572572
ap instanceof ApNil and
@@ -2080,7 +2080,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
20802080

20812081
override predicate isSource() {
20822082
sourceNode(node) and
2083-
(if hasSourceCallCtx() then cc = ccSomeCall() else cc = ccNone()) and
2083+
(if hasSourceCallCtx() then cc = ccSomeCall(true) else cc = ccNone()) and
20842084
summaryCtx.isSourceCtx() and
20852085
t = getNodeTyp(node) and
20862086
ap instanceof ApNil
@@ -2716,7 +2716,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
27162716

27172717
Cc ccNone() { result = false }
27182718

2719-
CcCall ccSomeCall() { result = true }
2719+
CcCall ccSomeCall(boolean isSource) { result = true and isSource = [false, true] }
27202720

27212721
predicate instanceofCc(Cc cc) { any() }
27222722

shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,10 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
494494
tgts = strictcount(Callable tgt | relevantCallEdgeIn(call, tgt)) and
495495
ctxtgts < tgts
496496
)
497+
or
498+
// If only a single lambda can reach `call`, we still want to check for the call
499+
// context, since lambdas outside the codebase may reach as well
500+
exists(viableCallableLambda(call, TCallSome(ctx)))
497501
}
498502

499503
/**
@@ -619,7 +623,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
619623
TSpecificCall(CallSet calls, DispatchSet tgts, UnreachableSetOption unreachable) {
620624
hasCtx(_, calls, tgts, unreachable)
621625
} or
622-
TSomeCall() or
626+
TSomeCall(Boolean isSource) or
623627
TReturn(Callable c, Call call) { reducedViableImplInReturn(c, call) }
624628

625629
/**
@@ -632,9 +636,10 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
632636
* dispatch targets for all of `calls` to the set of dispatch targets in
633637
* `tgts`, and/or the specific call prunes unreachable nodes in the
634638
* current callable as given by `unreachable`.
635-
* - `TSomeCall()` : Flow entered through a parameter. The
639+
* - `TSomeCall(boolean isSource)` : Flow entered through a parameter. The
636640
* originating call does not improve the set of dispatch targets for any
637-
* method call in the current callable and was therefore not recorded.
641+
* method call in the current callable and was therefore not recorded. `isSource`
642+
* indicates whether the call context is for a flow source when using `FlowFeature`s.
638643
* - `TReturn(Callable c, Call call)` : Flow reached `call` from `c` and
639644
* this dispatch target of `call` implies a reduced set of dispatch origins
640645
* to which data may flow if it should reach a `return` statement.
@@ -656,7 +661,13 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
656661
}
657662

658663
private class CallContextSomeCall extends CallContextCall, TSomeCall {
659-
override string toString() { result = "CcSomeCall" }
664+
private boolean isSource;
665+
666+
CallContextSomeCall() { this = TSomeCall(isSource) }
667+
668+
override string toString() {
669+
if isSource = true then result = "CcSomeCallSource" else result = "CcSomeCall"
670+
}
660671
}
661672

662673
private class CallContextReturn extends CallContextNoCall, TReturn {
@@ -707,7 +718,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
707718
pragma[inline]
708719
predicate matchesCall(CcCall cc, Call call) {
709720
cc = Input2::getSpecificCallContextCall(call, _) or
710-
cc = ccSomeCall()
721+
cc = ccSomeCall(false)
711722
}
712723

713724
class CcNoCall = CallContextNoCall;
@@ -716,7 +727,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
716727

717728
Cc ccNone() { result instanceof CallContextAny }
718729

719-
CcCall ccSomeCall() { result instanceof CallContextSomeCall }
730+
CcCall ccSomeCall(boolean isSource) { result = TSomeCall(isSource) }
720731

721732
predicate instanceofCc(Cc cc) { any() }
722733

@@ -739,7 +750,13 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
739750
/** Holds if `call` does not have a reduced set of dispatch targets in call context `ctx`. */
740751
bindingset[call, ctx]
741752
predicate viableImplNotCallContextReduced(Call call, CallContext ctx) {
742-
not Input2::callContextAffectsDispatch(call, ctx)
753+
not Input2::callContextAffectsDispatch(call, ctx) and
754+
// When sources have call contexts (using `FlowFeature`s), we check that `call` can
755+
// dispatch in all possible call contexts
756+
not (
757+
ctx = TSomeCall(true) and
758+
Input2::callContextAffectsDispatch(call, _)
759+
)
743760
}
744761

745762
/**
@@ -822,7 +839,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
822839
CallContextCall getCallContextCall(Call call, Callable c) {
823840
if recordCallSiteDispatch(call, c)
824841
then result = Input2::getSpecificCallContextCall(call, c)
825-
else result = TSomeCall()
842+
else result = TSomeCall(false)
826843
}
827844
}
828845

@@ -850,7 +867,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
850867
CallContextCall getCallContextCall(Call call, Callable c) {
851868
if recordCallSite(call, c)
852869
then result = Input2::getSpecificCallContextCall(call, c)
853-
else result = TSomeCall()
870+
else result = TSomeCall(false)
854871
}
855872
}
856873
}

0 commit comments

Comments
 (0)