Skip to content

Commit 5be75e9

Browse files
authored
Merge pull request #15796 from hvitved/csharp/variable-capture-follow-up
C#: Variable capture follow-up
2 parents a78e04e + a92e394 commit 5be75e9

File tree

6 files changed

+122
-97
lines changed

6 files changed

+122
-97
lines changed

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ abstract private class LocalFunctionCreationNode extends NodeImpl, TLocalFunctio
148148
LocalFunction getFunction() { result = function }
149149

150150
ExprNode getAnAccess(boolean inSameCallable) {
151-
result.getExpr().(LocalFunctionAccess).getTarget() = this.getFunction() and
151+
isLocalFunctionCallReceiver(_, result.getExpr(), this.getFunction()) and
152152
if result.getEnclosingCallable() = this.getEnclosingCallable()
153153
then inSameCallable = true
154154
else inSameCallable = false
@@ -399,7 +399,11 @@ module VariableCapture {
399399

400400
predicate hasBody(Callable body) { body = c }
401401

402-
predicate hasAliasedAccess(Expr f) { closureFlowStep+(this, f) and not closureFlowStep(f, _) }
402+
predicate hasAliasedAccess(Expr f) {
403+
closureFlowStep+(this, f) and not closureFlowStep(f, _)
404+
or
405+
isLocalFunctionCallReceiver(_, f.getAstNode(), c)
406+
}
403407
}
404408

405409
class Callable extends Cs::Callable {
@@ -881,7 +885,7 @@ module LocalFlow {
881885
exists(SsaImpl::getAReadAtNode(def, node2.(ExprNode).getControlFlowNode()))
882886
)
883887
or
884-
delegateCreationStep(node1, node2)
888+
node2 = node1.(LocalFunctionCreationNode).getAnAccess(true)
885889
or
886890
node1 =
887891
unique(FlowSummaryNode n1 |
@@ -2549,9 +2553,10 @@ class DataFlowType extends TDataFlowType {
25492553
* creations associated with the same type.
25502554
*/
25512555
ControlFlowElement getADelegateCreation() {
2552-
exists(Callable callable |
2553-
lambdaCreationExpr(result, callable) and
2554-
this = TDelegateDataFlowType(callable)
2556+
exists(Callable callable | this = TDelegateDataFlowType(callable) |
2557+
lambdaCreationExpr(result, callable)
2558+
or
2559+
isLocalFunctionCallReceiver(_, result, callable)
25552560
)
25562561
}
25572562

@@ -2566,12 +2571,7 @@ class DataFlowType extends TDataFlowType {
25662571
DataFlowType getNodeType(Node n) {
25672572
result = n.(NodeImpl).getDataFlowType() and
25682573
not lambdaCreation(n, _, _) and
2569-
not delegateCreationStep(_, n)
2570-
or
2571-
exists(Node arg |
2572-
delegateCreationStep(arg, n) and
2573-
result = getNodeType(arg)
2574-
)
2574+
not isLocalFunctionCallReceiver(_, n.asExpr(), _)
25752575
or
25762576
[
25772577
n.asExpr().(ControlFlowElement),
@@ -2896,7 +2896,7 @@ private predicate lambdaCreationExpr(ControlFlowElement creation, Callable c) {
28962896
c =
28972897
[
28982898
creation.(AnonymousFunctionExpr),
2899-
creation.(CallableAccess).getTarget().getUnboundDeclaration(),
2899+
creation.(DelegateCreation).getArgument().(CallableAccess).getTarget().getUnboundDeclaration(),
29002900
creation.(AddressOfExpr).getOperand().(CallableAccess).getTarget().getUnboundDeclaration(),
29012901
creation.(LocalFunctionStmt).getLocalFunction()
29022902
]
@@ -2910,6 +2910,13 @@ predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c)
29102910
exists(kind)
29112911
}
29122912

2913+
private predicate isLocalFunctionCallReceiver(
2914+
LocalFunctionCall call, LocalFunctionAccess receiver, LocalFunction f
2915+
) {
2916+
receiver.getParent() = call and
2917+
f = receiver.getTarget().getUnboundDeclaration()
2918+
}
2919+
29132920
private class LambdaConfiguration extends ControlFlowReachabilityConfiguration {
29142921
LambdaConfiguration() { this = "LambdaConfiguration" }
29152922

@@ -2926,7 +2933,7 @@ private class LambdaConfiguration extends ControlFlowReachabilityConfiguration {
29262933
scope = e2 and
29272934
isSuccessor = true
29282935
or
2929-
e1.(LocalFunctionAccess).getParent() = e2.(LocalFunctionCall) and
2936+
isLocalFunctionCallReceiver(e2, e1, _) and
29302937
exactScope = false and
29312938
scope = e2 and
29322939
isSuccessor = true

csharp/ql/test/library-tests/dataflow/global/Capture.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,9 +327,11 @@ void M12()
327327
{
328328
var x = "taint source";
329329

330-
void CapturedLocalFunction() => Check(x); // missing flow from line 328
330+
void CapturedLocalFunction() => Check(x);
331331

332332
void CapturingLocalFunction() => CapturedLocalFunction();
333+
334+
CapturingLocalFunction();
333335
}
334336

335337
void M13()

csharp/ql/test/library-tests/dataflow/global/DataFlow.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
| Capture.cs:312:15:312:15 | access to local variable x |
2828
| Capture.cs:319:19:319:19 | access to local variable x |
2929
| Capture.cs:330:47:330:47 | access to local variable x |
30-
| Capture.cs:339:45:339:45 | access to local variable x |
30+
| Capture.cs:341:45:341:45 | access to local variable x |
3131
| GlobalDataFlow.cs:19:15:19:29 | access to field SinkField0 |
3232
| GlobalDataFlow.cs:27:15:27:32 | access to property SinkProperty0 |
3333
| GlobalDataFlow.cs:45:50:45:59 | access to parameter sinkParam2 |

0 commit comments

Comments
 (0)