Skip to content

Commit 658388b

Browse files
committed
Python: Exclude sources in functions with unclear returns
A common source of FPs is when the flow inside a function depends on some argument to the function. In this case, if a non-container class is being returned in _some_ branch, we behave as if it _always_ is returned, leading to false positives where the code is actually safe because the argument to the function prevents the bad return from being executed.
1 parent c2c96b9 commit 658388b

File tree

1 file changed

+21
-1
lines changed

1 file changed

+21
-1
lines changed

python/ql/lib/semmle/python/dataflow/new/internal/ClassInstanceFlow.qll

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,22 @@ module ClassInstanceFlow<ClassInstanceFlowSig Sig> {
9797
node = DataFlow::BarrierGuard<isinstanceGuard/3>::getABarrierNode()
9898
}
9999

100+
/**
101+
* Holds if `call` is inside a branch that is guarded by a condition
102+
* depending on a parameter of the enclosing function. In such cases,
103+
* the instantiation is contextual — it only happens for certain argument
104+
* values — and we cannot determine from the call site whether it will
105+
* actually execute.
106+
*/
107+
private predicate parameterGuardedCall(CallNode call) {
108+
exists(ConditionBlock guard, DataFlow::ParameterNode param, DataFlow::Node guardSubExpr |
109+
guard.controls(call.getBasicBlock(), _) and
110+
param.getScope() = call.getScope() and
111+
guardSubExpr.asCfgNode() = guard.getLastNode().getAChild*() and
112+
DataFlow::localFlow(param, guardSubExpr)
113+
)
114+
}
115+
100116
predicate isAdditionalFlowStep(
101117
DataFlow::Node nodeFrom, FlowState stateFrom, DataFlow::Node nodeTo, FlowState stateTo
102118
) {
@@ -109,7 +125,11 @@ module ClassInstanceFlow<ClassInstanceFlowSig Sig> {
109125
nodeTo.asCfgNode() = call and
110126
// Exclude decorator applications, where the result is a proxy
111127
// rather than a typical instance.
112-
not call.getNode() = any(FunctionExpr fe).getADecoratorCall()
128+
not call.getNode() = any(FunctionExpr fe).getADecoratorCall() and
129+
// Exclude instantiations guarded by parameter-dependent conditions,
130+
// since we cannot determine from the call site whether the guard
131+
// will be satisfied.
132+
not parameterGuardedCall(call)
113133
)
114134
}
115135
}

0 commit comments

Comments
 (0)