Skip to content

C#: Handle CSV data-flow summaries with out/ref parameters #6152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hvitved merged 1 commit into from
Jun 25, 2021
Merged

C#: Handle CSV data-flow summaries with out/ref parameters #6152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 0 additions & 28 deletions csharp/ql/src/semmle/code/csharp/dataflow/FlowSummary.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,6 @@ module SummaryComponent {
/** Gets a summary component that represents the return value of a call. */
SummaryComponent return() { result = return(any(NormalReturnKind rk)) }

/**
* Gets a summary component that represents the return value through the `i`th
* `out` argument of a call.
*/
SummaryComponent outArgument(int i) {
result = return(any(OutReturnKind rk | rk.getPosition() = i))
}

/**
* Gets a summary component that represents the return value through the `i`th
* `ref` argument of a call.
*/
SummaryComponent refArgument(int i) {
result = return(any(RefReturnKind rk | rk.getPosition() = i))
}

/** Gets a summary component that represents a jump to `c`. */
SummaryComponent jump(Callable c) {
result =
Expand Down Expand Up @@ -88,18 +72,6 @@ module SummaryComponentStack {
/** Gets a singleton stack representing the return value of a call. */
SummaryComponentStack return() { result = singleton(SummaryComponent::return()) }

/**
* Gets a singleton stack representing the return value through the `i`th
* `out` argument of a call.
*/
SummaryComponentStack outArgument(int i) { result = singleton(SummaryComponent::outArgument(i)) }

/**
* Gets a singleton stack representing the return value through the `i`th
* `ref` argument of a call.
*/
SummaryComponentStack refArgument(int i) { result = singleton(SummaryComponent::refArgument(i)) }

/** Gets a singleton stack representing a jump to `c`. */
SummaryComponentStack jump(Callable c) { result = singleton(SummaryComponent::jump(c)) }
}
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/src/semmle/code/csharp/dataflow/LibraryTypeDataFlow.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ private module FrameworkDataFlowAdaptor {
or
exists(int i |
result = TCallableFlowSinkArg(i) and
output = SummaryComponentStack::outArgument(i)
output = SummaryComponentStack::argument(i)
)
or
exists(int i, int j | result = TCallableFlowSinkDelegateArg(i, j) |
Expand Down
26 changes: 25 additions & 1 deletion csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1258,12 +1258,33 @@ private module ReturnNodes {
SummaryReturnNode() {
FlowSummaryImpl::Private::summaryReturnNode(this, rk) and
not rk instanceof JumpReturnKind
or
exists(Parameter p, int pos |
summaryPostUpdateNodeIsOutOrRef(this, p) and
pos = p.getPosition()
|
p.isOut() and rk.(OutReturnKind).getPosition() = pos
or
p.isRef() and rk.(RefReturnKind).getPosition() = pos
)
}

override ReturnKind getKind() { result = rk }
}
}

/**
* Holds if summary node `n` is a post-update node for `out`/`ref` parameter `p`.
* In this case we adjust it to instead be a return node.
*/
private predicate summaryPostUpdateNodeIsOutOrRef(SummaryNode n, Parameter p) {
exists(ParameterNode pn |
FlowSummaryImpl::Private::summaryPostUpdateNode(n, pn) and
pn.getParameter() = p and
p.isOutOrRef()
)
}

import ReturnNodes

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

private class SummaryPostUpdateNode extends SummaryNode, PostUpdateNode {
SummaryPostUpdateNode() { FlowSummaryImpl::Private::summaryPostUpdateNode(this, _) }
SummaryPostUpdateNode() {
FlowSummaryImpl::Private::summaryPostUpdateNode(this, _) and
not summaryPostUpdateNodeIsOutOrRef(this, _)
}

override Node getPreUpdateNode() {
FlowSummaryImpl::Private::summaryPostUpdateNode(this, result)
Expand Down
9 changes: 9 additions & 0 deletions csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ void M13()
Sink(objs2[0]);
}

void M14()
{
var s = new string("");
Parse(s, out var i);
Sink(i);
}

object StepArgRes(object x) { return null; }

void StepArgArg(object @in, object @out) { }
Expand Down Expand Up @@ -115,6 +122,8 @@ void StepQualArg(object @out) { }

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

static void Parse(string s, out int i) => throw null;

static void Sink(object o) { }
}
}
20 changes: 14 additions & 6 deletions csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ edges
| ExternalFlow.cs:54:36:54:47 | object creation of type Object : Object | ExternalFlow.cs:54:13:54:16 | [post] this access [element] : Object |
| ExternalFlow.cs:55:18:55:21 | this access [element] : Object | ExternalFlow.cs:55:18:55:41 | call to method StepElementGetter |
| ExternalFlow.cs:60:35:60:35 | o : Object | ExternalFlow.cs:60:47:60:47 | access to parameter o |
| ExternalFlow.cs:60:64:60:75 | object creation of type Object : Object | ExternalFlow.cs:114:46:114:46 | s : Object |
| ExternalFlow.cs:60:64:60:75 | object creation of type Object : Object | ExternalFlow.cs:121:46:121:46 | s : Object |
| ExternalFlow.cs:65:21:65:60 | call to method Apply : Object | ExternalFlow.cs:66:18:66:18 | access to local variable o |
| ExternalFlow.cs:65:45:65:56 | object creation of type Object : Object | ExternalFlow.cs:65:21:65:60 | call to method Apply : Object |
| ExternalFlow.cs:71:30:71:45 | { ..., ... } [element] : Object | ExternalFlow.cs:72:17:72:20 | access to local variable objs [element] : Object |
| ExternalFlow.cs:71:32:71:43 | object creation of type Object : Object | ExternalFlow.cs:71:30:71:45 | { ..., ... } [element] : Object |
| ExternalFlow.cs:72:17:72:20 | access to local variable objs [element] : Object | ExternalFlow.cs:116:34:116:41 | elements [element] : Object |
| ExternalFlow.cs:72:17:72:20 | access to local variable objs [element] : Object | ExternalFlow.cs:123:34:123:41 | elements [element] : Object |
| ExternalFlow.cs:72:23:72:23 | o : Object | ExternalFlow.cs:72:35:72:35 | access to parameter o |
| 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 |
| ExternalFlow.cs:77:46:77:57 | object creation of type Object : Object | ExternalFlow.cs:77:24:77:58 | call to method Map [element] : Object |
Expand All @@ -40,8 +40,11 @@ edges
| 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 |
| 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 |
| ExternalFlow.cs:85:18:85:22 | access to local variable objs2 [element] : Object | ExternalFlow.cs:85:18:85:25 | access to array element |
| ExternalFlow.cs:114:46:114:46 | s : Object | ExternalFlow.cs:60:35:60:35 | o : Object |
| ExternalFlow.cs:116:34:116:41 | elements [element] : Object | ExternalFlow.cs:72:23:72:23 | o : Object |
| ExternalFlow.cs:90:21:90:34 | object creation of type String : String | ExternalFlow.cs:91:19:91:19 | access to local variable s : String |
| ExternalFlow.cs:91:19:91:19 | access to local variable s : String | ExternalFlow.cs:91:30:91:30 | SSA def(i) : Int32 |
| ExternalFlow.cs:91:30:91:30 | SSA def(i) : Int32 | ExternalFlow.cs:92:18:92:18 | (...) ... |
| ExternalFlow.cs:121:46:121:46 | s : Object | ExternalFlow.cs:60:35:60:35 | o : Object |
| ExternalFlow.cs:123:34:123:41 | elements [element] : Object | ExternalFlow.cs:72:23:72:23 | o : Object |
nodes
| ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
| ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | semmle.label | call to method StepArgRes |
Expand Down Expand Up @@ -97,8 +100,12 @@ nodes
| ExternalFlow.cs:84:29:84:32 | access to local variable objs [element] : Object | semmle.label | access to local variable objs [element] : Object |
| ExternalFlow.cs:85:18:85:22 | access to local variable objs2 [element] : Object | semmle.label | access to local variable objs2 [element] : Object |
| ExternalFlow.cs:85:18:85:25 | access to array element | semmle.label | access to array element |
| ExternalFlow.cs:114:46:114:46 | s : Object | semmle.label | s : Object |
| ExternalFlow.cs:116:34:116:41 | elements [element] : Object | semmle.label | elements [element] : Object |
| ExternalFlow.cs:90:21:90:34 | object creation of type String : String | semmle.label | object creation of type String : String |
| ExternalFlow.cs:91:19:91:19 | access to local variable s : String | semmle.label | access to local variable s : String |
| ExternalFlow.cs:91:30:91:30 | SSA def(i) : Int32 | semmle.label | SSA def(i) : Int32 |
| ExternalFlow.cs:92:18:92:18 | (...) ... | semmle.label | (...) ... |
| ExternalFlow.cs:121:46:121:46 | s : Object | semmle.label | s : Object |
| ExternalFlow.cs:123:34:123:41 | elements [element] : Object | semmle.label | elements [element] : Object |
invalidModelRow
#select
| 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 |
Expand All @@ -115,3 +122,4 @@ invalidModelRow
| 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 |
| 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 |
| 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 |
| 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 |
3 changes: 2 additions & 1 deletion csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ class SummaryModelTest extends SummaryModelCsv {
"My.Qltest;D;false;Apply;(System.Func<S,T>,S);;Argument[1];Parameter[0] of Argument[0];value",
"My.Qltest;D;false;Apply;(System.Func<S,T>,S);;ReturnValue of Argument[0];ReturnValue;value",
"My.Qltest;D;false;Map;(S[],System.Func<S,T>);;Element of Argument[0];Parameter[0] of Argument[1];value",
"My.Qltest;D;false;Map;(S[],System.Func<S,T>);;ReturnValue of Argument[1];Element of ReturnValue;value"
"My.Qltest;D;false;Map;(S[],System.Func<S,T>);;ReturnValue of Argument[1];Element of ReturnValue;value",
"My.Qltest;D;false;Parse;(System.String,System.Int32);;Argument[0];Argument[1];taint"
]
}
}
Expand Down
Loading