Skip to content

Commit 7a9f9e2

Browse files
hvitvedhvitved
committed
C#: Handle CSV data-flow summaries with out/ref parameters
1 parent 1e511c0 commit 7a9f9e2

File tree

9 files changed

+383
-368
lines changed

9 files changed

+383
-368
lines changed

‎csharp/ql/src/semmle/code/csharp/dataflow/FlowSummary.qll

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,6 @@ module SummaryComponent {
3535
/** Gets a summary component that represents the return value of a call. */
3636
SummaryComponent return() { result = return(any(NormalReturnKind rk)) }
3737

38-
/**
39-
* Gets a summary component that represents the return value through the `i`th
40-
* `out` argument of a call.
41-
*/
42-
SummaryComponent outArgument(int i) {
43-
result = return(any(OutReturnKind rk | rk.getPosition() = i))
44-
}
45-
46-
/**
47-
* Gets a summary component that represents the return value through the `i`th
48-
* `ref` argument of a call.
49-
*/
50-
SummaryComponent refArgument(int i) {
51-
result = return(any(RefReturnKind rk | rk.getPosition() = i))
52-
}
53-
5438
/** Gets a summary component that represents a jump to `c`. */
5539
SummaryComponent jump(Callable c) {
5640
result =
@@ -88,18 +72,6 @@ module SummaryComponentStack {
8872
/** Gets a singleton stack representing the return value of a call. */
8973
SummaryComponentStack return() { result = singleton(SummaryComponent::return()) }
9074

91-
/**
92-
* Gets a singleton stack representing the return value through the `i`th
93-
* `out` argument of a call.
94-
*/
95-
SummaryComponentStack outArgument(int i) { result = singleton(SummaryComponent::outArgument(i)) }
96-
97-
/**
98-
* Gets a singleton stack representing the return value through the `i`th
99-
* `ref` argument of a call.
100-
*/
101-
SummaryComponentStack refArgument(int i) { result = singleton(SummaryComponent::refArgument(i)) }
102-
10375
/** Gets a singleton stack representing a jump to `c`. */
10476
SummaryComponentStack jump(Callable c) { result = singleton(SummaryComponent::jump(c)) }
10577
}

‎csharp/ql/src/semmle/code/csharp/dataflow/LibraryTypeDataFlow.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ private module FrameworkDataFlowAdaptor {
383383
or
384384
exists(int i |
385385
result = TCallableFlowSinkArg(i) and
386-
output = SummaryComponentStack::outArgument(i)
386+
output = SummaryComponentStack::argument(i)
387387
)
388388
or
389389
exists(int i, int j | result = TCallableFlowSinkDelegateArg(i, j) |

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1258,12 +1258,33 @@ private module ReturnNodes {
12581258
SummaryReturnNode() {
12591259
FlowSummaryImpl::Private::summaryReturnNode(this, rk) and
12601260
not rk instanceof JumpReturnKind
1261+
or
1262+
exists(Parameter p, int pos |
1263+
summaryPostUpdateNodeIsOutOrRef(this, p) and
1264+
pos = p.getPosition()
1265+
|
1266+
p.isOut() and rk.(OutReturnKind).getPosition() = pos
1267+
or
1268+
p.isRef() and rk.(RefReturnKind).getPosition() = pos
1269+
)
12611270
}
12621271

12631272
override ReturnKind getKind() { result = rk }
12641273
}
12651274
}
12661275

1276+
/**
1277+
* Holds if summary node `n` is a post-update node for `out`/`ref` parameter `p`.
1278+
* In this case we adjust it to instead be a return node.
1279+
*/
1280+
private predicate summaryPostUpdateNodeIsOutOrRef(SummaryNode n, Parameter p) {
1281+
exists(ParameterNode pn |
1282+
FlowSummaryImpl::Private::summaryPostUpdateNode(n, pn) and
1283+
pn.getParameter() = p and
1284+
p.isOutOrRef()
1285+
)
1286+
}
1287+
12671288
import ReturnNodes
12681289

12691290
/** A data-flow node that represents the output of a call. */
@@ -1841,7 +1862,10 @@ private module PostUpdateNodes {
18411862
}
18421863

18431864
private class SummaryPostUpdateNode extends SummaryNode, PostUpdateNode {
1844-
SummaryPostUpdateNode() { FlowSummaryImpl::Private::summaryPostUpdateNode(this, _) }
1865+
SummaryPostUpdateNode() {
1866+
FlowSummaryImpl::Private::summaryPostUpdateNode(this, _) and
1867+
not summaryPostUpdateNodeIsOutOrRef(this, _)
1868+
}
18451869

18461870
override Node getPreUpdateNode() {
18471871
FlowSummaryImpl::Private::summaryPostUpdateNode(this, result)

‎csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,13 @@ void M13()
8585
Sink(objs2[0]);
8686
}
8787

88+
void M14()
89+
{
90+
var s = new string("");
91+
Parse(s, out var i);
92+
Sink(i);
93+
}
94+
8895
object StepArgRes(object x) { return null; }
8996

9097
void StepArgArg(object @in, object @out) { }
@@ -115,6 +122,8 @@ void StepQualArg(object @out) { }
115122

116123
static S[] Map<S, T>(S[] elements, Func<S, T> f) => throw null;
117124

125+
static void Parse(string s, out int i) => throw null;
126+
118127
static void Sink(object o) { }
119128
}
120129
}

‎csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ edges
2424
| ExternalFlow.cs:54:36:54:47 | object creation of type Object : Object | ExternalFlow.cs:54:13:54:16 | [post] this access [element] : Object |
2525
| ExternalFlow.cs:55:18:55:21 | this access [element] : Object | ExternalFlow.cs:55:18:55:41 | call to method StepElementGetter |
2626
| ExternalFlow.cs:60:35:60:35 | o : Object | ExternalFlow.cs:60:47:60:47 | access to parameter o |
27-
| ExternalFlow.cs:60:64:60:75 | object creation of type Object : Object | ExternalFlow.cs:114:46:114:46 | s : Object |
27+
| ExternalFlow.cs:60:64:60:75 | object creation of type Object : Object | ExternalFlow.cs:121:46:121:46 | s : Object |
2828
| ExternalFlow.cs:65:21:65:60 | call to method Apply : Object | ExternalFlow.cs:66:18:66:18 | access to local variable o |
2929
| ExternalFlow.cs:65:45:65:56 | object creation of type Object : Object | ExternalFlow.cs:65:21:65:60 | call to method Apply : Object |
3030
| ExternalFlow.cs:71:30:71:45 | { ..., ... } [element] : Object | ExternalFlow.cs:72:17:72:20 | access to local variable objs [element] : Object |
3131
| ExternalFlow.cs:71:32:71:43 | object creation of type Object : Object | ExternalFlow.cs:71:30:71:45 | { ..., ... } [element] : Object |
32-
| ExternalFlow.cs:72:17:72:20 | access to local variable objs [element] : Object | ExternalFlow.cs:116:34:116:41 | elements [element] : Object |
32+
| ExternalFlow.cs:72:17:72:20 | access to local variable objs [element] : Object | ExternalFlow.cs:123:34:123:41 | elements [element] : Object |
3333
| ExternalFlow.cs:72:23:72:23 | o : Object | ExternalFlow.cs:72:35:72:35 | access to parameter o |
3434
| ExternalFlow.cs:77:24:77:58 | call to method Map [element] : Object | ExternalFlow.cs:78:18:78:21 | access to local variable objs [element] : Object |
3535
| ExternalFlow.cs:77:46:77:57 | object creation of type Object : Object | ExternalFlow.cs:77:24:77:58 | call to method Map [element] : Object |
@@ -40,8 +40,11 @@ edges
4040
| ExternalFlow.cs:84:25:84:41 | call to method Map [element] : Object | ExternalFlow.cs:85:18:85:22 | access to local variable objs2 [element] : Object |
4141
| ExternalFlow.cs:84:29:84:32 | access to local variable objs [element] : Object | ExternalFlow.cs:84:25:84:41 | call to method Map [element] : Object |
4242
| ExternalFlow.cs:85:18:85:22 | access to local variable objs2 [element] : Object | ExternalFlow.cs:85:18:85:25 | access to array element |
43-
| ExternalFlow.cs:114:46:114:46 | s : Object | ExternalFlow.cs:60:35:60:35 | o : Object |
44-
| ExternalFlow.cs:116:34:116:41 | elements [element] : Object | ExternalFlow.cs:72:23:72:23 | o : Object |
43+
| ExternalFlow.cs:90:21:90:34 | object creation of type String : String | ExternalFlow.cs:91:19:91:19 | access to local variable s : String |
44+
| ExternalFlow.cs:91:19:91:19 | access to local variable s : String | ExternalFlow.cs:91:30:91:30 | SSA def(i) : Int32 |
45+
| ExternalFlow.cs:91:30:91:30 | SSA def(i) : Int32 | ExternalFlow.cs:92:18:92:18 | (...) ... |
46+
| ExternalFlow.cs:121:46:121:46 | s : Object | ExternalFlow.cs:60:35:60:35 | o : Object |
47+
| ExternalFlow.cs:123:34:123:41 | elements [element] : Object | ExternalFlow.cs:72:23:72:23 | o : Object |
4548
nodes
4649
| ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
4750
| ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | semmle.label | call to method StepArgRes |
@@ -97,8 +100,12 @@ nodes
97100
| ExternalFlow.cs:84:29:84:32 | access to local variable objs [element] : Object | semmle.label | access to local variable objs [element] : Object |
98101
| ExternalFlow.cs:85:18:85:22 | access to local variable objs2 [element] : Object | semmle.label | access to local variable objs2 [element] : Object |
99102
| ExternalFlow.cs:85:18:85:25 | access to array element | semmle.label | access to array element |
100-
| ExternalFlow.cs:114:46:114:46 | s : Object | semmle.label | s : Object |
101-
| ExternalFlow.cs:116:34:116:41 | elements [element] : Object | semmle.label | elements [element] : Object |
103+
| ExternalFlow.cs:90:21:90:34 | object creation of type String : String | semmle.label | object creation of type String : String |
104+
| ExternalFlow.cs:91:19:91:19 | access to local variable s : String | semmle.label | access to local variable s : String |
105+
| ExternalFlow.cs:91:30:91:30 | SSA def(i) : Int32 | semmle.label | SSA def(i) : Int32 |
106+
| ExternalFlow.cs:92:18:92:18 | (...) ... | semmle.label | (...) ... |
107+
| ExternalFlow.cs:121:46:121:46 | s : Object | semmle.label | s : Object |
108+
| ExternalFlow.cs:123:34:123:41 | elements [element] : Object | semmle.label | elements [element] : Object |
102109
invalidModelRow
103110
#select
104111
| ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | $@ | ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | object creation of type Object : Object |
@@ -115,3 +122,4 @@ invalidModelRow
115122
| ExternalFlow.cs:72:35:72:35 | access to parameter o | ExternalFlow.cs:71:32:71:43 | object creation of type Object : Object | ExternalFlow.cs:72:35:72:35 | access to parameter o | $@ | ExternalFlow.cs:71:32:71:43 | object creation of type Object : Object | object creation of type Object : Object |
116123
| ExternalFlow.cs:78:18:78:24 | (...) ... | ExternalFlow.cs:77:46:77:57 | object creation of type Object : Object | ExternalFlow.cs:78:18:78:24 | (...) ... | $@ | ExternalFlow.cs:77:46:77:57 | object creation of type Object : Object | object creation of type Object : Object |
117124
| ExternalFlow.cs:85:18:85:25 | access to array element | ExternalFlow.cs:83:32:83:43 | object creation of type Object : Object | ExternalFlow.cs:85:18:85:25 | access to array element | $@ | ExternalFlow.cs:83:32:83:43 | object creation of type Object : Object | object creation of type Object : Object |
125+
| ExternalFlow.cs:92:18:92:18 | (...) ... | ExternalFlow.cs:90:21:90:34 | object creation of type String : String | ExternalFlow.cs:92:18:92:18 | (...) ... | $@ | ExternalFlow.cs:90:21:90:34 | object creation of type String : String | object creation of type String : String |

‎csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ class SummaryModelTest extends SummaryModelCsv {
2424
"My.Qltest;D;false;Apply;(System.Func<S,T>,S);;Argument[1];Parameter[0] of Argument[0];value",
2525
"My.Qltest;D;false;Apply;(System.Func<S,T>,S);;ReturnValue of Argument[0];ReturnValue;value",
2626
"My.Qltest;D;false;Map;(S[],System.Func<S,T>);;Element of Argument[0];Parameter[0] of Argument[1];value",
27-
"My.Qltest;D;false;Map;(S[],System.Func<S,T>);;ReturnValue of Argument[1];Element of ReturnValue;value"
27+
"My.Qltest;D;false;Map;(S[],System.Func<S,T>);;ReturnValue of Argument[1];Element of ReturnValue;value",
28+
"My.Qltest;D;false;Parse;(System.String,System.Int32);;Argument[0];Argument[1];taint"
2829
]
2930
}
3031
}

0 commit comments

Comments
 (0)