Skip to content

Data flow: Restrict derived flow summaries #6931

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
aschackmull merged 6 commits into from
Nov 18, 2021
Merged

Data flow: Restrict derived flow summaries #6931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0c1285f
Data flow: Restrict derived flow summaries
hvitved Sep 30, 2021
ac41451
Data flow: Sync files
hvitved Oct 21, 2021
58dd758
C#: Update flow summary to avoid negative recursion
hvitved Oct 21, 2021
3da73b9
C#: Update expected test output
hvitved Oct 21, 2021
6d58dd2
Java: Update expected test output
hvitved Oct 21, 2021
4eacbd1
Ruby: Sync files
hvitved Nov 2, 2021
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
105 changes: 82 additions & 23 deletions csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ module Public {
/** Holds if this stack contains summary component `c`. */
predicate contains(SummaryComponent c) { c = this.drop(_).head() }

/** Gets the bottom element of this stack. */
SummaryComponent bottom() { result = this.drop(this.length() - 1).head() }

/** Gets a textual representation of this stack. */
string toString() {
exists(SummaryComponent head, SummaryComponentStack tail |
Expand Down Expand Up @@ -197,6 +200,8 @@ module Private {
or
tail.(RequiredSummaryComponentStack).required(TParameterSummaryComponent(_)) and
head = thisParam()
or
derivedFluentFlowPush(_, _, _, head, tail, _)
}

pragma[nomagic]
Expand All @@ -210,7 +215,7 @@ module Private {
c.propagatesFlow(output, input, preservesValue) and
preservesValue = true and
isCallbackParameter(input) and
isContentOfArgument(output)
isContentOfArgument(output, _)
or
// flow from the receiver of a callback into the instance-parameter
exists(SummaryComponentStack s, SummaryComponentStack callbackRef |
Expand All @@ -222,16 +227,81 @@ module Private {
output = TConsSummaryComponentStack(thisParam(), input) and
preservesValue = true
)
or
exists(SummaryComponentStack arg, SummaryComponentStack return |
derivedFluentFlow(c, input, arg, return, preservesValue)
|
arg.length() = 1 and
output = return
or
exists(SummaryComponent head, SummaryComponentStack tail |
derivedFluentFlowPush(c, input, arg, head, tail, 0) and
output = SummaryComponentStack::push(head, tail)
)
)
or
// Chain together summaries where values get passed into callbacks along the way
exists(SummaryComponentStack mid, boolean preservesValue1, boolean preservesValue2 |
c.propagatesFlow(input, mid, preservesValue1) and
c.propagatesFlow(mid, output, preservesValue2) and
mid.drop(mid.length() - 2) =
SummaryComponentStack::push(TParameterSummaryComponent(_),
SummaryComponentStack::singleton(TArgumentSummaryComponent(_))) and
preservesValue = preservesValue1.booleanAnd(preservesValue2)
)
}

/**
* Holds if `c` has a flow summary from `input` to `arg`, where `arg`
* writes to (contents of) the `i`th argument, and `c` has a
* value-preserving flow summary from the `i`th argument to a return value
* (`return`).
*
* In such a case, we derive flow from `input` to (contents of) the return
* value.
*
* As an example, this simplifies modeling of fluent methods:
* for `StringBuilder.append(x)` with a specified value flow from qualifier to
* return value and taint flow from argument 0 to the qualifier, then this
* allows us to infer taint flow from argument 0 to the return value.
*/
pragma[nomagic]
private predicate derivedFluentFlow(
SummarizedCallable c, SummaryComponentStack input, SummaryComponentStack arg,
SummaryComponentStack return, boolean preservesValue
) {
exists(int i |
summary(c, input, arg, preservesValue) and
isContentOfArgument(arg, i) and
summary(c, SummaryComponentStack::singleton(TArgumentSummaryComponent(i)), return, true) and
return.bottom() = TReturnSummaryComponent(_)
)
}

pragma[nomagic]
private predicate derivedFluentFlowPush(
SummarizedCallable c, SummaryComponentStack input, SummaryComponentStack arg,
SummaryComponent head, SummaryComponentStack tail, int i
) {
derivedFluentFlow(c, input, arg, tail, _) and
head = arg.drop(i).head() and
i = arg.length() - 2
or
exists(SummaryComponent head0, SummaryComponentStack tail0 |
derivedFluentFlowPush(c, input, arg, head0, tail0, i + 1) and
head = arg.drop(i).head() and
tail = SummaryComponentStack::push(head0, tail0)
)
}

private predicate isCallbackParameter(SummaryComponentStack s) {
s.head() = TParameterSummaryComponent(_) and exists(s.tail())
}

private predicate isContentOfArgument(SummaryComponentStack s) {
s.head() = TContentSummaryComponent(_) and isContentOfArgument(s.tail())
private predicate isContentOfArgument(SummaryComponentStack s, int i) {
s.head() = TContentSummaryComponent(_) and isContentOfArgument(s.tail(), i)
or
s = TSingletonSummaryComponentStack(TArgumentSummaryComponent(_))
s = TSingletonSummaryComponentStack(TArgumentSummaryComponent(i))
}

private predicate outputState(SummarizedCallable c, SummaryComponentStack s) {
Expand Down Expand Up @@ -508,9 +578,14 @@ module Private {
* node, and back out to `p`.
*/
predicate summaryAllowParameterReturnInSelf(ParamNode p) {
exists(SummarizedCallable c, int i |
c.clearsContent(i, _) and
p.isParameterOf(c, i)
exists(SummarizedCallable c, int i | p.isParameterOf(c, i) |
c.clearsContent(i, _)
or
exists(SummaryComponentStack inputContents, SummaryComponentStack outputContents |
summary(c, inputContents, outputContents, _) and
inputContents.bottom() = pragma[only_bind_into](TArgumentSummaryComponent(i)) and
outputContents.bottom() = pragma[only_bind_into](TArgumentSummaryComponent(i))
)
)
}

Expand All @@ -534,22 +609,6 @@ module Private {
preservesValue = false and not summary(c, inputContents, outputContents, true)
)
or
// If flow through a method updates a parameter from some input A, and that
// parameter also is returned through B, then we'd like a combined flow from A
// to B as well. As an example, this simplifies modeling of fluent methods:
// for `StringBuilder.append(x)` with a specified value flow from qualifier to
// return value and taint flow from argument 0 to the qualifier, then this
// allows us to infer taint flow from argument 0 to the return value.
succ instanceof ParamNode and
summaryPostUpdateNode(pred, succ) and
preservesValue = true
or
// Similarly we would like to chain together summaries where values get passed
// into callbacks along the way.
pred instanceof ArgNode and
summaryPostUpdateNode(succ, pred) and
preservesValue = true
or
exists(SummarizedCallable c, int i |
pred.(ParamNode).isParameterOf(c, i) and
succ = summaryNode(c, TSummaryNodeClearsContentState(i, _)) and
Expand Down
38 changes: 20 additions & 18 deletions csharp/ql/lib/semmle/code/csharp/frameworks/EntityFramework.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,32 +174,34 @@ module EntityFramework {
}
}

private class RawSqlStringSummarizedCallable extends EFSummarizedCallable {
private SummaryComponentStack input_;
private SummaryComponentStack output_;
private boolean preservesValue_;

RawSqlStringSummarizedCallable() {
private class RawSqlStringConstructorSummarizedCallable extends EFSummarizedCallable {
RawSqlStringConstructorSummarizedCallable() {
exists(RawSqlStringStruct s |
this = s.getAConstructor() and
input_ = SummaryComponentStack::argument(0) and
this.getNumberOfParameters() > 0 and
output_ = SummaryComponentStack::return() and
preservesValue_ = false
or
this = s.getAConversionTo() and
input_ = SummaryComponentStack::argument(0) and
output_ = SummaryComponentStack::return() and
preservesValue_ = false
this.getNumberOfParameters() > 0
)
}

override predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
input = input_ and
output = output_ and
preservesValue = preservesValue_
input = SummaryComponentStack::argument(0) and
output = SummaryComponentStack::return() and
preservesValue = false
}
}

private class RawSqlStringConversionSummarizedCallable extends EFSummarizedCallable {
RawSqlStringConversionSummarizedCallable() {
exists(RawSqlStringStruct s | this = s.getAConversionTo())
}

override predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
input = SummaryComponentStack::argument(0) and
output = SummaryComponentStack::return() and
preservesValue = false
}
}

Expand Down
6 changes: 3 additions & 3 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 @@ -33,7 +33,7 @@ void M4()

void M5()
{
this.StepFieldSetter(new object());
Sink(((D)this.StepFieldSetter(new object()).Field2).Field);
Sink(this.Field);
}

Expand Down Expand Up @@ -102,7 +102,7 @@ void M15()
Sink(d);
}, d1, d2);
Sink(d1.Field);
Sink(d2.Field2); // SPURIOUS FLOW
Sink(d2.Field2);
}

object StepArgRes(object x) { return null; }
Expand All @@ -120,7 +120,7 @@ void StepQualArg(object @out) { }

object StepFieldGetter() => throw null;

void StepFieldSetter(object value) => throw null;
D StepFieldSetter(object value) => throw null;

object Property { get; set; }

Expand Down
26 changes: 14 additions & 12 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 @@ -11,8 +11,12 @@ edges
| ExternalFlow.cs:30:13:30:16 | [post] this access [field Field] : Object | ExternalFlow.cs:31:18:31:21 | this access [field Field] : Object |
| ExternalFlow.cs:30:26:30:37 | object creation of type Object : Object | ExternalFlow.cs:30:13:30:16 | [post] this access [field Field] : Object |
| ExternalFlow.cs:31:18:31:21 | this access [field Field] : Object | ExternalFlow.cs:31:18:31:39 | call to method StepFieldGetter |
| ExternalFlow.cs:36:13:36:16 | [post] this access [field Field] : Object | ExternalFlow.cs:37:18:37:21 | this access [field Field] : Object |
| ExternalFlow.cs:36:34:36:45 | object creation of type Object : Object | ExternalFlow.cs:36:13:36:16 | [post] this access [field Field] : Object |
| ExternalFlow.cs:36:19:36:62 | (...) ... [field Field] : Object | ExternalFlow.cs:36:18:36:69 | access to field Field |
| ExternalFlow.cs:36:22:36:25 | [post] this access [field Field] : Object | ExternalFlow.cs:37:18:37:21 | this access [field Field] : Object |
| ExternalFlow.cs:36:22:36:55 | call to method StepFieldSetter [field Field2, field Field] : Object | ExternalFlow.cs:36:22:36:62 | access to field Field2 [field Field] : Object |
| ExternalFlow.cs:36:22:36:62 | access to field Field2 [field Field] : Object | ExternalFlow.cs:36:19:36:62 | (...) ... [field Field] : Object |
| ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | ExternalFlow.cs:36:22:36:25 | [post] this access [field Field] : Object |
| ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | ExternalFlow.cs:36:22:36:55 | call to method StepFieldSetter [field Field2, field Field] : Object |
| ExternalFlow.cs:37:18:37:21 | this access [field Field] : Object | ExternalFlow.cs:37:18:37:27 | access to field Field |
| ExternalFlow.cs:42:13:42:16 | [post] this access [property Property] : Object | ExternalFlow.cs:43:18:43:21 | this access [property Property] : Object |
| ExternalFlow.cs:42:29:42:40 | object creation of type Object : Object | ExternalFlow.cs:42:13:42:16 | [post] this access [property Property] : Object |
Expand Down Expand Up @@ -48,10 +52,7 @@ edges
| ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | ExternalFlow.cs:98:13:98:14 | [post] access to local variable d1 [field Field] : Object |
| ExternalFlow.cs:100:20:100:20 | d : Object | ExternalFlow.cs:102:22:102:22 | access to parameter d |
| ExternalFlow.cs:103:16:103:17 | access to local variable d1 [field Field] : Object | ExternalFlow.cs:100:20:100:20 | d : Object |
| ExternalFlow.cs:103:16:103:17 | access to local variable d1 [field Field] : Object | ExternalFlow.cs:103:20:103:21 | [post] access to local variable d2 [field Field2] : Object |
| ExternalFlow.cs:103:20:103:21 | [post] access to local variable d2 [field Field2] : Object | ExternalFlow.cs:105:18:105:19 | access to local variable d2 [field Field2] : Object |
| ExternalFlow.cs:104:18:104:19 | access to local variable d1 [field Field] : Object | ExternalFlow.cs:104:18:104:25 | access to field Field |
| ExternalFlow.cs:105:18:105:19 | access to local variable d2 [field Field2] : Object | ExternalFlow.cs:105:18:105:26 | access to field Field2 |
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 All @@ -69,8 +70,12 @@ nodes
| ExternalFlow.cs:30:26:30:37 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
| ExternalFlow.cs:31:18:31:21 | this access [field Field] : Object | semmle.label | this access [field Field] : Object |
| ExternalFlow.cs:31:18:31:39 | call to method StepFieldGetter | semmle.label | call to method StepFieldGetter |
| ExternalFlow.cs:36:13:36:16 | [post] this access [field Field] : Object | semmle.label | [post] this access [field Field] : Object |
| ExternalFlow.cs:36:34:36:45 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
| ExternalFlow.cs:36:18:36:69 | access to field Field | semmle.label | access to field Field |
| ExternalFlow.cs:36:19:36:62 | (...) ... [field Field] : Object | semmle.label | (...) ... [field Field] : Object |
| ExternalFlow.cs:36:22:36:25 | [post] this access [field Field] : Object | semmle.label | [post] this access [field Field] : Object |
| ExternalFlow.cs:36:22:36:55 | call to method StepFieldSetter [field Field2, field Field] : Object | semmle.label | call to method StepFieldSetter [field Field2, field Field] : Object |
| ExternalFlow.cs:36:22:36:62 | access to field Field2 [field Field] : Object | semmle.label | access to field Field2 [field Field] : Object |
| ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
| ExternalFlow.cs:37:18:37:21 | this access [field Field] : Object | semmle.label | this access [field Field] : Object |
| ExternalFlow.cs:37:18:37:27 | access to field Field | semmle.label | access to field Field |
| ExternalFlow.cs:42:13:42:16 | [post] this access [property Property] : Object | semmle.label | [post] this access [property Property] : Object |
Expand Down Expand Up @@ -116,11 +121,8 @@ nodes
| ExternalFlow.cs:100:20:100:20 | d : Object | semmle.label | d : Object |
| ExternalFlow.cs:102:22:102:22 | access to parameter d | semmle.label | access to parameter d |
| ExternalFlow.cs:103:16:103:17 | access to local variable d1 [field Field] : Object | semmle.label | access to local variable d1 [field Field] : Object |
| ExternalFlow.cs:103:20:103:21 | [post] access to local variable d2 [field Field2] : Object | semmle.label | [post] access to local variable d2 [field Field2] : Object |
| ExternalFlow.cs:104:18:104:19 | access to local variable d1 [field Field] : Object | semmle.label | access to local variable d1 [field Field] : Object |
| ExternalFlow.cs:104:18:104:25 | access to field Field | semmle.label | access to field Field |
| ExternalFlow.cs:105:18:105:19 | access to local variable d2 [field Field2] : Object | semmle.label | access to local variable d2 [field Field2] : Object |
| ExternalFlow.cs:105:18:105:26 | access to field Field2 | semmle.label | access to field Field2 |
subpaths
invalidModelRow
#select
Expand All @@ -129,7 +131,8 @@ invalidModelRow
| ExternalFlow.cs:18:18:18:24 | access to local variable argOut1 | ExternalFlow.cs:16:30:16:41 | object creation of type Object : Object | ExternalFlow.cs:18:18:18:24 | access to local variable argOut1 | $@ | ExternalFlow.cs:16:30:16:41 | object creation of type Object : Object | object creation of type Object : Object |
| ExternalFlow.cs:25:18:25:21 | this access | ExternalFlow.cs:23:27:23:38 | object creation of type Object : Object | ExternalFlow.cs:25:18:25:21 | this access | $@ | ExternalFlow.cs:23:27:23:38 | object creation of type Object : Object | object creation of type Object : Object |
| ExternalFlow.cs:31:18:31:39 | call to method StepFieldGetter | ExternalFlow.cs:30:26:30:37 | object creation of type Object : Object | ExternalFlow.cs:31:18:31:39 | call to method StepFieldGetter | $@ | ExternalFlow.cs:30:26:30:37 | object creation of type Object : Object | object creation of type Object : Object |
| ExternalFlow.cs:37:18:37:27 | access to field Field | ExternalFlow.cs:36:34:36:45 | object creation of type Object : Object | ExternalFlow.cs:37:18:37:27 | access to field Field | $@ | ExternalFlow.cs:36:34:36:45 | object creation of type Object : Object | object creation of type Object : Object |
| ExternalFlow.cs:36:18:36:69 | access to field Field | ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | ExternalFlow.cs:36:18:36:69 | access to field Field | $@ | ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | object creation of type Object : Object |
| ExternalFlow.cs:37:18:37:27 | access to field Field | ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | ExternalFlow.cs:37:18:37:27 | access to field Field | $@ | ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | object creation of type Object : Object |
| ExternalFlow.cs:43:18:43:42 | call to method StepPropertyGetter | ExternalFlow.cs:42:29:42:40 | object creation of type Object : Object | ExternalFlow.cs:43:18:43:42 | call to method StepPropertyGetter | $@ | ExternalFlow.cs:42:29:42:40 | object creation of type Object : Object | object creation of type Object : Object |
| ExternalFlow.cs:49:18:49:30 | access to property Property | ExternalFlow.cs:48:37:48:48 | object creation of type Object : Object | ExternalFlow.cs:49:18:49:30 | access to property Property | $@ | ExternalFlow.cs:48:37:48:48 | object creation of type Object : Object | object creation of type Object : Object |
| ExternalFlow.cs:55:18:55:41 | call to method StepElementGetter | ExternalFlow.cs:54:36:54:47 | object creation of type Object : Object | ExternalFlow.cs:55:18:55:41 | call to method StepElementGetter | $@ | ExternalFlow.cs:54:36:54:47 | object creation of type Object : Object | object creation of type Object : Object |
Expand All @@ -141,4 +144,3 @@ invalidModelRow
| 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 |
| ExternalFlow.cs:102:22:102:22 | access to parameter d | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | ExternalFlow.cs:102:22:102:22 | access to parameter d | $@ | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | object creation of type Object : Object |
| ExternalFlow.cs:104:18:104:25 | access to field Field | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | ExternalFlow.cs:104:18:104:25 | access to field Field | $@ | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | object creation of type Object : Object |
| ExternalFlow.cs:105:18:105:26 | access to field Field2 | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | ExternalFlow.cs:105:18:105:26 | access to field Field2 | $@ | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | object creation of type Object : Object |
1 change: 1 addition & 0 deletions 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 @@ -17,6 +17,7 @@ class SummaryModelTest extends SummaryModelCsv {
"My.Qltest;D;false;StepArgQual;(System.Object);;Argument[0];Argument[-1];taint",
"My.Qltest;D;false;StepFieldGetter;();;Field[My.Qltest.D.Field] of Argument[-1];ReturnValue;value",
"My.Qltest;D;false;StepFieldSetter;(System.Object);;Argument[0];Field[My.Qltest.D.Field] of Argument[-1];value",
"My.Qltest;D;false;StepFieldSetter;(System.Object);;Argument[-1];Field[My.Qltest.D.Field2] of ReturnValue;value",
"My.Qltest;D;false;StepPropertyGetter;();;Property[My.Qltest.D.Property] of Argument[-1];ReturnValue;value",
"My.Qltest;D;false;StepPropertySetter;(System.Object);;Argument[0];Property[My.Qltest.D.Property] of Argument[-1];value",
"My.Qltest;D;false;StepElementGetter;();;Element of Argument[-1];ReturnValue;value",
Expand Down
Loading