Skip to content

Commit 0e0f78e

Browse files
hvitvedhvitved
authored
Merge pull request github#1943 from lcartey/csharp/ttransitivecapture-fix
C#: Include runtime target in TTransitiveCaptureCall
2 parents e314a2c + 407f634 commit 0e0f78e

File tree

10 files changed

+1282
-994
lines changed

10 files changed

+1282
-994
lines changed

‎csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ private module Cached {
9797
TImplicitDelegateCall(ControlFlow::Nodes::ElementNode cfn, DelegateArgumentToLibraryCallable arg) {
9898
cfn.getElement() = arg
9999
} or
100-
TTransitiveCapturedCall(ControlFlow::Nodes::ElementNode cfn) {
101-
transitiveCapturedCallTarget(cfn, _)
100+
TTransitiveCapturedCall(ControlFlow::Nodes::ElementNode cfn, Callable target) {
101+
transitiveCapturedCallTarget(cfn, target)
102102
} or
103103
TCilCall(CIL::Call call) {
104104
// No need to include calls that are compiled from source
@@ -418,10 +418,11 @@ class ImplicitDelegateDataFlowCall extends DelegateDataFlowCall, TImplicitDelega
418418
*/
419419
class TransitiveCapturedDataFlowCall extends DataFlowCall, TTransitiveCapturedCall {
420420
private ControlFlow::Nodes::ElementNode cfn;
421+
private Callable target;
421422

422-
TransitiveCapturedDataFlowCall() { this = TTransitiveCapturedCall(cfn) }
423+
TransitiveCapturedDataFlowCall() { this = TTransitiveCapturedCall(cfn, target) }
423424

424-
override Callable getARuntimeTarget() { transitiveCapturedCallTarget(cfn, result) }
425+
override Callable getARuntimeTarget() { result = target }
425426

426427
override ControlFlow::Nodes::ElementNode getControlFlowNode() { result = cfn }
427428

‎csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -953,7 +953,7 @@ private module OutNodes {
953953
additionalCalls = false and
954954
call.(ImplicitDelegateDataFlowCall).isArgumentOf(csharpCall(_, cfn), _)
955955
or
956-
additionalCalls = true and call = TTransitiveCapturedCall(cfn)
956+
additionalCalls = true and call = TTransitiveCapturedCall(cfn, n.getEnclosingCallable())
957957
)
958958
}
959959

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,18 @@ void M()
4747
};
4848
};
4949
CaptureIn2NotCalled();
50+
void CaptureTest(string nonSink0, string sink39)
51+
{
52+
RunAction(() => // Check each lambda captures the correct arguments
53+
{
54+
Check(nonSink0);
55+
RunAction(() =>
56+
{
57+
Check(sink39);
58+
});
59+
});
60+
}
61+
CaptureTest("not tainted", tainted);
5062
}
5163

5264
void Out()
@@ -96,6 +108,18 @@ void M()
96108
};
97109
CaptureOut2NotCalled();
98110
Check(nonSink0);
111+
string sink40 = "";
112+
void CaptureOutMultipleLambdas()
113+
{
114+
RunAction(() => {
115+
sink40 = "taint source";
116+
});
117+
RunAction(() => {
118+
nonSink0 = "not tainted";
119+
});
120+
};
121+
CaptureOutMultipleLambdas();
122+
Check(sink40); Check(nonSink0);
99123
}
100124

101125
void Through(string tainted)
@@ -174,4 +198,9 @@ string Id(string s)
174198
}
175199

176200
static void Check<T>(T x) { }
201+
202+
static void RunAction(Action a)
203+
{
204+
a.Invoke();
205+
}
177206
}

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
| Capture.cs:12:19:12:24 | access to local variable sink27 |
22
| Capture.cs:21:23:21:28 | access to local variable sink28 |
33
| Capture.cs:30:19:30:24 | access to local variable sink29 |
4-
| Capture.cs:60:15:60:20 | access to local variable sink30 |
5-
| Capture.cs:72:15:72:20 | access to local variable sink31 |
6-
| Capture.cs:81:15:81:20 | access to local variable sink32 |
7-
| Capture.cs:109:15:109:20 | access to local variable sink33 |
8-
| Capture.cs:121:15:121:20 | access to local variable sink34 |
9-
| Capture.cs:130:15:130:20 | access to local variable sink35 |
10-
| Capture.cs:137:15:137:20 | access to local variable sink36 |
11-
| Capture.cs:145:15:145:20 | access to local variable sink37 |
12-
| Capture.cs:171:15:171:20 | access to local variable sink38 |
4+
| Capture.cs:57:27:57:32 | access to parameter sink39 |
5+
| Capture.cs:72:15:72:20 | access to local variable sink30 |
6+
| Capture.cs:84:15:84:20 | access to local variable sink31 |
7+
| Capture.cs:93:15:93:20 | access to local variable sink32 |
8+
| Capture.cs:122:15:122:20 | access to local variable sink40 |
9+
| Capture.cs:133:15:133:20 | access to local variable sink33 |
10+
| Capture.cs:145:15:145:20 | access to local variable sink34 |
11+
| Capture.cs:154:15:154:20 | access to local variable sink35 |
12+
| Capture.cs:161:15:161:20 | access to local variable sink36 |
13+
| Capture.cs:169:15:169:20 | access to local variable sink37 |
14+
| Capture.cs:195:15:195:20 | access to local variable sink38 |
1315
| GlobalDataFlow.cs:18:15:18:29 | access to field SinkField0 |
1416
| GlobalDataFlow.cs:26:15:26:32 | access to property SinkProperty0 |
1517
| GlobalDataFlow.cs:44:50:44:59 | access to parameter sinkParam2 |

‎csharp/ql/test/library-tests/dataflow/global/DataFlowEdges.expected

Lines changed: 513 additions & 406 deletions
Large diffs are not rendered by default.

‎csharp/ql/test/library-tests/dataflow/global/DataFlowPath.expected

Lines changed: 46 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,39 +2,47 @@ edges
22
| Capture.cs:7:20:7:26 | tainted | Capture.cs:14:9:14:20 | [implicit argument] tainted |
33
| Capture.cs:7:20:7:26 | tainted | Capture.cs:25:9:25:20 | [implicit argument] tainted |
44
| Capture.cs:7:20:7:26 | tainted | Capture.cs:33:9:33:40 | [implicit argument] tainted |
5+
| Capture.cs:7:20:7:26 | tainted | Capture.cs:61:36:61:42 | access to parameter tainted |
56
| Capture.cs:9:9:13:9 | SSA capture def(tainted) | Capture.cs:12:19:12:24 | access to local variable sink27 |
67
| Capture.cs:14:9:14:20 | [implicit argument] tainted | Capture.cs:9:9:13:9 | SSA capture def(tainted) |
78
| Capture.cs:18:13:22:13 | SSA capture def(tainted) | Capture.cs:21:23:21:28 | access to local variable sink28 |
89
| Capture.cs:25:9:25:20 | [implicit argument] tainted | Capture.cs:18:13:22:13 | SSA capture def(tainted) |
910
| Capture.cs:27:43:32:9 | SSA capture def(tainted) | Capture.cs:30:19:30:24 | access to local variable sink29 |
1011
| Capture.cs:33:9:33:40 | [implicit argument] tainted | Capture.cs:27:43:32:9 | SSA capture def(tainted) |
11-
| Capture.cs:57:13:57:35 | SSA def(sink30) | Capture.cs:59:9:59:21 | SSA call def(sink30) |
12-
| Capture.cs:57:22:57:35 | "taint source" | Capture.cs:57:13:57:35 | SSA def(sink30) |
13-
| Capture.cs:59:9:59:21 | SSA call def(sink30) | Capture.cs:60:15:60:20 | access to local variable sink30 |
14-
| Capture.cs:67:17:67:39 | SSA def(sink31) | Capture.cs:71:9:71:21 | SSA call def(sink31) |
15-
| Capture.cs:67:26:67:39 | "taint source" | Capture.cs:67:17:67:39 | SSA def(sink31) |
16-
| Capture.cs:71:9:71:21 | SSA call def(sink31) | Capture.cs:72:15:72:20 | access to local variable sink31 |
17-
| Capture.cs:77:13:77:35 | SSA def(sink32) | Capture.cs:80:9:80:41 | SSA call def(sink32) |
18-
| Capture.cs:77:22:77:35 | "taint source" | Capture.cs:77:13:77:35 | SSA def(sink32) |
19-
| Capture.cs:80:9:80:41 | SSA call def(sink32) | Capture.cs:81:15:81:20 | access to local variable sink32 |
20-
| Capture.cs:101:25:101:31 | tainted | Capture.cs:108:9:108:25 | [implicit argument] tainted |
21-
| Capture.cs:101:25:101:31 | tainted | Capture.cs:120:9:120:25 | [implicit argument] tainted |
22-
| Capture.cs:101:25:101:31 | tainted | Capture.cs:129:9:129:45 | [implicit argument] tainted |
23-
| Capture.cs:101:25:101:31 | tainted | Capture.cs:136:22:136:38 | [implicit argument] tainted |
24-
| Capture.cs:101:25:101:31 | tainted | Capture.cs:144:25:144:31 | access to parameter tainted |
25-
| Capture.cs:101:25:101:31 | tainted | Capture.cs:170:25:170:31 | access to parameter tainted |
26-
| Capture.cs:108:9:108:25 | SSA call def(sink33) | Capture.cs:109:15:109:20 | access to local variable sink33 |
27-
| Capture.cs:108:9:108:25 | [implicit argument] tainted | Capture.cs:108:9:108:25 | SSA call def(sink33) |
28-
| Capture.cs:120:9:120:25 | SSA call def(sink34) | Capture.cs:121:15:121:20 | access to local variable sink34 |
29-
| Capture.cs:120:9:120:25 | [implicit argument] tainted | Capture.cs:120:9:120:25 | SSA call def(sink34) |
30-
| Capture.cs:129:9:129:45 | SSA call def(sink35) | Capture.cs:130:15:130:20 | access to local variable sink35 |
31-
| Capture.cs:129:9:129:45 | [implicit argument] tainted | Capture.cs:129:9:129:45 | SSA call def(sink35) |
32-
| Capture.cs:136:22:136:38 | [implicit argument] tainted | Capture.cs:136:22:136:38 | call to local function CaptureThrough4 |
33-
| Capture.cs:136:22:136:38 | call to local function CaptureThrough4 | Capture.cs:137:15:137:20 | access to local variable sink36 |
34-
| Capture.cs:144:9:144:32 | SSA call def(sink37) | Capture.cs:145:15:145:20 | access to local variable sink37 |
35-
| Capture.cs:144:25:144:31 | access to parameter tainted | Capture.cs:144:9:144:32 | SSA call def(sink37) |
36-
| Capture.cs:170:22:170:32 | call to local function Id | Capture.cs:171:15:171:20 | access to local variable sink38 |
37-
| Capture.cs:170:25:170:31 | access to parameter tainted | Capture.cs:170:22:170:32 | call to local function Id |
12+
| Capture.cs:50:50:50:55 | sink39 | Capture.cs:52:13:59:14 | [implicit argument] sink39 |
13+
| Capture.cs:52:13:59:14 | [implicit argument] sink39 | Capture.cs:55:27:58:17 | SSA capture def(sink39) |
14+
| Capture.cs:55:27:58:17 | SSA capture def(sink39) | Capture.cs:57:27:57:32 | access to parameter sink39 |
15+
| Capture.cs:61:36:61:42 | access to parameter tainted | Capture.cs:50:50:50:55 | sink39 |
16+
| Capture.cs:69:13:69:35 | SSA def(sink30) | Capture.cs:71:9:71:21 | SSA call def(sink30) |
17+
| Capture.cs:69:22:69:35 | "taint source" | Capture.cs:69:13:69:35 | SSA def(sink30) |
18+
| Capture.cs:71:9:71:21 | SSA call def(sink30) | Capture.cs:72:15:72:20 | access to local variable sink30 |
19+
| Capture.cs:79:17:79:39 | SSA def(sink31) | Capture.cs:83:9:83:21 | SSA call def(sink31) |
20+
| Capture.cs:79:26:79:39 | "taint source" | Capture.cs:79:17:79:39 | SSA def(sink31) |
21+
| Capture.cs:83:9:83:21 | SSA call def(sink31) | Capture.cs:84:15:84:20 | access to local variable sink31 |
22+
| Capture.cs:89:13:89:35 | SSA def(sink32) | Capture.cs:92:9:92:41 | SSA call def(sink32) |
23+
| Capture.cs:89:22:89:35 | "taint source" | Capture.cs:89:13:89:35 | SSA def(sink32) |
24+
| Capture.cs:92:9:92:41 | SSA call def(sink32) | Capture.cs:93:15:93:20 | access to local variable sink32 |
25+
| Capture.cs:115:17:115:39 | SSA def(sink40) | Capture.cs:121:9:121:35 | SSA call def(sink40) |
26+
| Capture.cs:115:26:115:39 | "taint source" | Capture.cs:115:17:115:39 | SSA def(sink40) |
27+
| Capture.cs:121:9:121:35 | SSA call def(sink40) | Capture.cs:122:15:122:20 | access to local variable sink40 |
28+
| Capture.cs:125:25:125:31 | tainted | Capture.cs:132:9:132:25 | [implicit argument] tainted |
29+
| Capture.cs:125:25:125:31 | tainted | Capture.cs:144:9:144:25 | [implicit argument] tainted |
30+
| Capture.cs:125:25:125:31 | tainted | Capture.cs:153:9:153:45 | [implicit argument] tainted |
31+
| Capture.cs:125:25:125:31 | tainted | Capture.cs:160:22:160:38 | [implicit argument] tainted |
32+
| Capture.cs:125:25:125:31 | tainted | Capture.cs:168:25:168:31 | access to parameter tainted |
33+
| Capture.cs:125:25:125:31 | tainted | Capture.cs:194:25:194:31 | access to parameter tainted |
34+
| Capture.cs:132:9:132:25 | SSA call def(sink33) | Capture.cs:133:15:133:20 | access to local variable sink33 |
35+
| Capture.cs:132:9:132:25 | [implicit argument] tainted | Capture.cs:132:9:132:25 | SSA call def(sink33) |
36+
| Capture.cs:144:9:144:25 | SSA call def(sink34) | Capture.cs:145:15:145:20 | access to local variable sink34 |
37+
| Capture.cs:144:9:144:25 | [implicit argument] tainted | Capture.cs:144:9:144:25 | SSA call def(sink34) |
38+
| Capture.cs:153:9:153:45 | SSA call def(sink35) | Capture.cs:154:15:154:20 | access to local variable sink35 |
39+
| Capture.cs:153:9:153:45 | [implicit argument] tainted | Capture.cs:153:9:153:45 | SSA call def(sink35) |
40+
| Capture.cs:160:22:160:38 | [implicit argument] tainted | Capture.cs:160:22:160:38 | call to local function CaptureThrough4 |
41+
| Capture.cs:160:22:160:38 | call to local function CaptureThrough4 | Capture.cs:161:15:161:20 | access to local variable sink36 |
42+
| Capture.cs:168:9:168:32 | SSA call def(sink37) | Capture.cs:169:15:169:20 | access to local variable sink37 |
43+
| Capture.cs:168:25:168:31 | access to parameter tainted | Capture.cs:168:9:168:32 | SSA call def(sink37) |
44+
| Capture.cs:194:22:194:32 | call to local function Id | Capture.cs:195:15:195:20 | access to local variable sink38 |
45+
| Capture.cs:194:25:194:31 | access to parameter tainted | Capture.cs:194:22:194:32 | call to local function Id |
3846
| GlobalDataFlow.cs:17:27:17:40 | "taint source" | GlobalDataFlow.cs:18:15:18:29 | access to field SinkField0 |
3947
| GlobalDataFlow.cs:17:27:17:40 | "taint source" | GlobalDataFlow.cs:26:15:26:32 | access to property SinkProperty0 |
4048
| GlobalDataFlow.cs:17:27:17:40 | "taint source" | GlobalDataFlow.cs:26:15:26:32 | access to property SinkProperty0 |
@@ -210,23 +218,25 @@ edges
210218
| Capture.cs:21:23:21:28 | access to local variable sink28 | Capture.cs:7:20:7:26 | tainted | Capture.cs:21:23:21:28 | access to local variable sink28 | access to local variable sink28 |
211219
| Capture.cs:30:19:30:24 | access to local variable sink29 | Capture.cs:7:20:7:26 | tainted | Capture.cs:30:19:30:24 | access to local variable sink29 | access to local variable sink29 |
212220
| GlobalDataFlow.cs:79:15:79:19 | access to local variable sink3 | GlobalDataFlow.cs:17:27:17:40 | "taint source" | GlobalDataFlow.cs:79:15:79:19 | access to local variable sink3 | access to local variable sink3 |
213-
| Capture.cs:60:15:60:20 | access to local variable sink30 | Capture.cs:57:22:57:35 | "taint source" | Capture.cs:60:15:60:20 | access to local variable sink30 | access to local variable sink30 |
214-
| Capture.cs:72:15:72:20 | access to local variable sink31 | Capture.cs:67:26:67:39 | "taint source" | Capture.cs:72:15:72:20 | access to local variable sink31 | access to local variable sink31 |
215-
| Capture.cs:81:15:81:20 | access to local variable sink32 | Capture.cs:77:22:77:35 | "taint source" | Capture.cs:81:15:81:20 | access to local variable sink32 | access to local variable sink32 |
216-
| Capture.cs:109:15:109:20 | access to local variable sink33 | Capture.cs:101:25:101:31 | tainted | Capture.cs:109:15:109:20 | access to local variable sink33 | access to local variable sink33 |
217-
| Capture.cs:121:15:121:20 | access to local variable sink34 | Capture.cs:101:25:101:31 | tainted | Capture.cs:121:15:121:20 | access to local variable sink34 | access to local variable sink34 |
218-
| Capture.cs:130:15:130:20 | access to local variable sink35 | Capture.cs:101:25:101:31 | tainted | Capture.cs:130:15:130:20 | access to local variable sink35 | access to local variable sink35 |
219-
| Capture.cs:137:15:137:20 | access to local variable sink36 | Capture.cs:101:25:101:31 | tainted | Capture.cs:137:15:137:20 | access to local variable sink36 | access to local variable sink36 |
220-
| Capture.cs:145:15:145:20 | access to local variable sink37 | Capture.cs:101:25:101:31 | tainted | Capture.cs:145:15:145:20 | access to local variable sink37 | access to local variable sink37 |
221-
| Capture.cs:171:15:171:20 | access to local variable sink38 | Capture.cs:101:25:101:31 | tainted | Capture.cs:171:15:171:20 | access to local variable sink38 | access to local variable sink38 |
221+
| Capture.cs:72:15:72:20 | access to local variable sink30 | Capture.cs:69:22:69:35 | "taint source" | Capture.cs:72:15:72:20 | access to local variable sink30 | access to local variable sink30 |
222+
| Capture.cs:84:15:84:20 | access to local variable sink31 | Capture.cs:79:26:79:39 | "taint source" | Capture.cs:84:15:84:20 | access to local variable sink31 | access to local variable sink31 |
223+
| Capture.cs:93:15:93:20 | access to local variable sink32 | Capture.cs:89:22:89:35 | "taint source" | Capture.cs:93:15:93:20 | access to local variable sink32 | access to local variable sink32 |
224+
| Capture.cs:133:15:133:20 | access to local variable sink33 | Capture.cs:125:25:125:31 | tainted | Capture.cs:133:15:133:20 | access to local variable sink33 | access to local variable sink33 |
225+
| Capture.cs:145:15:145:20 | access to local variable sink34 | Capture.cs:125:25:125:31 | tainted | Capture.cs:145:15:145:20 | access to local variable sink34 | access to local variable sink34 |
226+
| Capture.cs:154:15:154:20 | access to local variable sink35 | Capture.cs:125:25:125:31 | tainted | Capture.cs:154:15:154:20 | access to local variable sink35 | access to local variable sink35 |
227+
| Capture.cs:161:15:161:20 | access to local variable sink36 | Capture.cs:125:25:125:31 | tainted | Capture.cs:161:15:161:20 | access to local variable sink36 | access to local variable sink36 |
228+
| Capture.cs:169:15:169:20 | access to local variable sink37 | Capture.cs:125:25:125:31 | tainted | Capture.cs:169:15:169:20 | access to local variable sink37 | access to local variable sink37 |
229+
| Capture.cs:195:15:195:20 | access to local variable sink38 | Capture.cs:125:25:125:31 | tainted | Capture.cs:195:15:195:20 | access to local variable sink38 | access to local variable sink38 |
222230
| GlobalDataFlow.cs:136:15:136:19 | access to local variable sink4 | GlobalDataFlow.cs:17:27:17:40 | "taint source" | GlobalDataFlow.cs:136:15:136:19 | access to local variable sink4 | access to local variable sink4 |
231+
| Capture.cs:122:15:122:20 | access to local variable sink40 | Capture.cs:115:26:115:39 | "taint source" | Capture.cs:122:15:122:20 | access to local variable sink40 | access to local variable sink40 |
223232
| GlobalDataFlow.cs:144:15:144:19 | access to local variable sink5 | GlobalDataFlow.cs:17:27:17:40 | "taint source" | GlobalDataFlow.cs:144:15:144:19 | access to local variable sink5 | access to local variable sink5 |
224233
| GlobalDataFlow.cs:154:15:154:19 | access to local variable sink6 | GlobalDataFlow.cs:318:16:318:29 | "taint source" | GlobalDataFlow.cs:154:15:154:19 | access to local variable sink6 | access to local variable sink6 |
225234
| GlobalDataFlow.cs:157:15:157:19 | access to local variable sink7 | GlobalDataFlow.cs:323:13:323:26 | "taint source" | GlobalDataFlow.cs:157:15:157:19 | access to local variable sink7 | access to local variable sink7 |
226235
| GlobalDataFlow.cs:160:15:160:19 | access to local variable sink8 | GlobalDataFlow.cs:328:13:328:26 | "taint source" | GlobalDataFlow.cs:160:15:160:19 | access to local variable sink8 | access to local variable sink8 |
227236
| GlobalDataFlow.cs:181:15:181:19 | access to local variable sink9 | GlobalDataFlow.cs:179:35:179:48 | "taint source" | GlobalDataFlow.cs:181:15:181:19 | access to local variable sink9 | access to local variable sink9 |
228237
| Splitting.cs:11:19:11:19 | access to local variable x | Splitting.cs:3:28:3:34 | tainted | Splitting.cs:11:19:11:19 | access to local variable x | access to local variable x |
229238
| Splitting.cs:34:19:34:19 | access to local variable x | Splitting.cs:24:28:24:34 | tainted | Splitting.cs:34:19:34:19 | access to local variable x | access to local variable x |
239+
| Capture.cs:57:27:57:32 | access to parameter sink39 | Capture.cs:7:20:7:26 | tainted | Capture.cs:57:27:57:32 | access to parameter sink39 | access to parameter sink39 |
230240
| GlobalDataFlow.cs:237:15:237:24 | access to parameter sinkParam0 | GlobalDataFlow.cs:17:27:17:40 | "taint source" | GlobalDataFlow.cs:237:15:237:24 | access to parameter sinkParam0 | access to parameter sinkParam0 |
231241
| GlobalDataFlow.cs:242:15:242:24 | access to parameter sinkParam1 | GlobalDataFlow.cs:17:27:17:40 | "taint source" | GlobalDataFlow.cs:242:15:242:24 | access to parameter sinkParam1 | access to parameter sinkParam1 |
232242
| GlobalDataFlow.cs:44:50:44:59 | access to parameter sinkParam2 | GlobalDataFlow.cs:17:27:17:40 | "taint source" | GlobalDataFlow.cs:44:50:44:59 | access to parameter sinkParam2 | access to parameter sinkParam2 |

0 commit comments

Comments
 (0)