Skip to content

Commit 10739b1

Browse files
hvitvedhvitved
authored
Merge pull request github#6841 from hvitved/dataflow/incorrect-summary-chaining
Data flow: Add tests for missing summary flow
2 parents 6b4ca31 + cc305ed commit 10739b1

File tree

9 files changed

+259
-10
lines changed

9 files changed

+259
-10
lines changed

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,19 @@ abstract class DataFlowCall extends TDataFlowCall {
262262

263263
/** Gets the location of this call. */
264264
abstract Location getLocation();
265+
266+
/**
267+
* Holds if this element is at the specified location.
268+
* The location spans column `startcolumn` of line `startline` to
269+
* column `endcolumn` of line `endline` in file `filepath`.
270+
* For more information, see
271+
* [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
272+
*/
273+
final predicate hasLocationInfo(
274+
string filepath, int startline, int startcolumn, int endline, int endcolumn
275+
) {
276+
this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
277+
}
265278
}
266279

267280
/** A non-delegate C# call relevant for data flow. */

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -870,4 +870,95 @@ module Private {
870870
)
871871
}
872872
}
873+
874+
/**
875+
* Provides query predicates for rendering the generated data flow graph for
876+
* a summarized callable.
877+
*
878+
* Import this module into a `.ql` file of `@kind graph` to render the graph.
879+
* The graph is restricted to callables from `RelevantSummarizedCallable`.
880+
*/
881+
module RenderSummarizedCallable {
882+
/** A summarized callable to include in the graph. */
883+
abstract class RelevantSummarizedCallable extends SummarizedCallable { }
884+
885+
private newtype TNodeOrCall =
886+
MkNode(Node n) {
887+
exists(RelevantSummarizedCallable c |
888+
n = summaryNode(c, _)
889+
or
890+
n.(ParamNode).isParameterOf(c, _)
891+
)
892+
} or
893+
MkCall(DataFlowCall call) {
894+
call = summaryDataFlowCall(_) and
895+
call.getEnclosingCallable() instanceof RelevantSummarizedCallable
896+
}
897+
898+
private class NodeOrCall extends TNodeOrCall {
899+
Node asNode() { this = MkNode(result) }
900+
901+
DataFlowCall asCall() { this = MkCall(result) }
902+
903+
string toString() {
904+
result = this.asNode().toString()
905+
or
906+
result = this.asCall().toString()
907+
}
908+
909+
/**
910+
* Holds if this element is at the specified location.
911+
* The location spans column `startcolumn` of line `startline` to
912+
* column `endcolumn` of line `endline` in file `filepath`.
913+
* For more information, see
914+
* [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
915+
*/
916+
predicate hasLocationInfo(
917+
string filepath, int startline, int startcolumn, int endline, int endcolumn
918+
) {
919+
this.asNode().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
920+
or
921+
this.asCall().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
922+
}
923+
}
924+
925+
query predicate nodes(NodeOrCall n, string key, string val) {
926+
key = "semmle.label" and val = n.toString()
927+
}
928+
929+
private predicate edgesComponent(NodeOrCall a, NodeOrCall b, string value) {
930+
exists(boolean preservesValue |
931+
Private::Steps::summaryLocalStep(a.asNode(), b.asNode(), preservesValue) and
932+
if preservesValue = true then value = "value" else value = "taint"
933+
)
934+
or
935+
exists(Content c |
936+
Private::Steps::summaryReadStep(a.asNode(), c, b.asNode()) and
937+
value = "read (" + c + ")"
938+
or
939+
Private::Steps::summaryStoreStep(a.asNode(), c, b.asNode()) and
940+
value = "store (" + c + ")"
941+
or
942+
Private::Steps::summaryClearsContent(a.asNode(), c) and
943+
b = a and
944+
value = "clear (" + c + ")"
945+
)
946+
or
947+
summaryPostUpdateNode(b.asNode(), a.asNode()) and
948+
value = "post-update"
949+
or
950+
b.asCall() = summaryDataFlowCall(a.asNode()) and
951+
value = "receiver"
952+
or
953+
exists(int i |
954+
summaryArgumentNode(b.asCall(), a.asNode(), i) and
955+
value = "argument (" + i + ")"
956+
)
957+
}
958+
959+
query predicate edges(NodeOrCall a, NodeOrCall b, string key, string value) {
960+
key = "semmle.label" and
961+
value = strictconcat(string s | edgesComponent(a, b, s) | s, " / ")
962+
}
963+
}
873964
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,19 @@ void M14()
9292
Sink(i);
9393
}
9494

95+
void M15()
96+
{
97+
var d1 = new D();
98+
d1.Field = new object();
99+
var d2 = new D();
100+
Apply2(d =>
101+
{
102+
Sink(d); // MISSING FLOW
103+
}, d1, d2);
104+
Sink(d1.Field); // MISSING FLOW
105+
Sink(d2.Field2);
106+
}
107+
95108
object StepArgRes(object x) { return null; }
96109

97110
void StepArgArg(object @in, object @out) { }
@@ -103,6 +116,7 @@ void StepArgQual(object x) { }
103116
void StepQualArg(object @out) { }
104117

105118
object Field;
119+
object Field2;
106120

107121
object StepFieldGetter() => throw null;
108122

@@ -122,6 +136,8 @@ void StepQualArg(object @out) { }
122136

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

139+
static void Apply2<S>(Action<S> f, S s1, S s2) => throw null;
140+
125141
static void Parse(string s, out int i) => throw null;
126142

127143
static void Sink(object o) { }

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

Lines changed: 10 additions & 10 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:121:46:121:46 | s : Object |
27+
| ExternalFlow.cs:60:64:60:75 | object creation of type Object : Object | ExternalFlow.cs:135:46:135:46 | s : Object |
2828
| ExternalFlow.cs:65:21:65:60 | call to method Apply<Int32,Object> : 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<Int32,Object> : 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:123:34:123:41 | elements [element] : Object |
32+
| ExternalFlow.cs:72:17:72:20 | access to local variable objs [element] : Object | ExternalFlow.cs:137:34:137: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<Int32,Object> [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<Int32,Object> [element] : Object |
@@ -43,11 +43,11 @@ edges
4343
| ExternalFlow.cs:90:21:90:34 | object creation of type String : String | ExternalFlow.cs:91:19:91:19 | access to local variable s : String |
4444
| ExternalFlow.cs:91:19:91:19 | access to local variable s : String | ExternalFlow.cs:91:30:91:30 | SSA def(i) : Int32 |
4545
| 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 |
48-
| ExternalFlow.cs:123:34:123:41 | elements [element] : Object | ExternalFlow.cs:72:23:72:23 | o : Object |
49-
| ExternalFlow.cs:123:34:123:41 | elements [element] : Object | ExternalFlow.cs:123:34:123:41 | elements [element] : Object |
50-
| ExternalFlow.cs:123:34:123:41 | elements [element] : Object | ExternalFlow.cs:123:34:123:41 | elements [element] : Object |
46+
| ExternalFlow.cs:135:46:135:46 | s : Object | ExternalFlow.cs:60:35:60:35 | o : Object |
47+
| ExternalFlow.cs:137:34:137:41 | elements [element] : Object | ExternalFlow.cs:72:23:72:23 | o : Object |
48+
| ExternalFlow.cs:137:34:137:41 | elements [element] : Object | ExternalFlow.cs:72:23:72:23 | o : Object |
49+
| ExternalFlow.cs:137:34:137:41 | elements [element] : Object | ExternalFlow.cs:137:34:137:41 | elements [element] : Object |
50+
| ExternalFlow.cs:137:34:137:41 | elements [element] : Object | ExternalFlow.cs:137:34:137:41 | elements [element] : Object |
5151
nodes
5252
| ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
5353
| ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | semmle.label | call to method StepArgRes |
@@ -107,9 +107,9 @@ nodes
107107
| ExternalFlow.cs:91:19:91:19 | access to local variable s : String | semmle.label | access to local variable s : String |
108108
| ExternalFlow.cs:91:30:91:30 | SSA def(i) : Int32 | semmle.label | SSA def(i) : Int32 |
109109
| ExternalFlow.cs:92:18:92:18 | (...) ... | semmle.label | (...) ... |
110-
| ExternalFlow.cs:121:46:121:46 | s : Object | semmle.label | s : Object |
111-
| ExternalFlow.cs:123:34:123:41 | elements [element] : Object | semmle.label | elements [element] : Object |
112-
| ExternalFlow.cs:123:34:123:41 | elements [element] : Object | semmle.label | elements [element] : Object |
110+
| ExternalFlow.cs:135:46:135:46 | s : Object | semmle.label | s : Object |
111+
| ExternalFlow.cs:137:34:137:41 | elements [element] : Object | semmle.label | elements [element] : Object |
112+
| ExternalFlow.cs:137:34:137:41 | elements [element] : Object | semmle.label | elements [element] : Object |
113113
subpaths
114114
invalidModelRow
115115
#select

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ class SummaryModelTest extends SummaryModelCsv {
2323
"My.Qltest;D;false;StepElementSetter;(System.Object);;Argument[0];Element of Argument[-1];value",
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",
26+
"My.Qltest;D;false;Apply2<>;(System.Action<S>,S,S);;Field[My.Qltest.D.Field] of Argument[1];Parameter[0] of Argument[0];value",
27+
"My.Qltest;D;false;Apply2<>;(System.Action<S>,S,S);;Field[My.Qltest.D.Field2] of Argument[2];Parameter[0] of Argument[0];value",
2628
"My.Qltest;D;false;Map<,>;(S[],System.Func<S,T>);;Element of Argument[0];Parameter[0] of Argument[1];value",
2729
"My.Qltest;D;false;Map<,>;(S[],System.Func<S,T>);;ReturnValue of Argument[1];Element of ReturnValue;value",
2830
"My.Qltest;D;false;Parse;(System.String,System.Int32);;Argument[0];Argument[1];taint"

‎java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,19 @@ class DataFlowCall extends TDataFlowCall {
234234

235235
/** Gets the location of this call. */
236236
abstract Location getLocation();
237+
238+
/**
239+
* Holds if this element is at the specified location.
240+
* The location spans column `startcolumn` of line `startline` to
241+
* column `endcolumn` of line `endline` in file `filepath`.
242+
* For more information, see
243+
* [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
244+
*/
245+
final predicate hasLocationInfo(
246+
string filepath, int startline, int startcolumn, int endline, int endcolumn
247+
) {
248+
this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
249+
}
237250
}
238251

239252
/** A source call, that is, a `Call`. */

‎java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -870,4 +870,95 @@ module Private {
870870
)
871871
}
872872
}
873+
874+
/**
875+
* Provides query predicates for rendering the generated data flow graph for
876+
* a summarized callable.
877+
*
878+
* Import this module into a `.ql` file of `@kind graph` to render the graph.
879+
* The graph is restricted to callables from `RelevantSummarizedCallable`.
880+
*/
881+
module RenderSummarizedCallable {
882+
/** A summarized callable to include in the graph. */
883+
abstract class RelevantSummarizedCallable extends SummarizedCallable { }
884+
885+
private newtype TNodeOrCall =
886+
MkNode(Node n) {
887+
exists(RelevantSummarizedCallable c |
888+
n = summaryNode(c, _)
889+
or
890+
n.(ParamNode).isParameterOf(c, _)
891+
)
892+
} or
893+
MkCall(DataFlowCall call) {
894+
call = summaryDataFlowCall(_) and
895+
call.getEnclosingCallable() instanceof RelevantSummarizedCallable
896+
}
897+
898+
private class NodeOrCall extends TNodeOrCall {
899+
Node asNode() { this = MkNode(result) }
900+
901+
DataFlowCall asCall() { this = MkCall(result) }
902+
903+
string toString() {
904+
result = this.asNode().toString()
905+
or
906+
result = this.asCall().toString()
907+
}
908+
909+
/**
910+
* Holds if this element is at the specified location.
911+
* The location spans column `startcolumn` of line `startline` to
912+
* column `endcolumn` of line `endline` in file `filepath`.
913+
* For more information, see
914+
* [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
915+
*/
916+
predicate hasLocationInfo(
917+
string filepath, int startline, int startcolumn, int endline, int endcolumn
918+
) {
919+
this.asNode().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
920+
or
921+
this.asCall().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
922+
}
923+
}
924+
925+
query predicate nodes(NodeOrCall n, string key, string val) {
926+
key = "semmle.label" and val = n.toString()
927+
}
928+
929+
private predicate edgesComponent(NodeOrCall a, NodeOrCall b, string value) {
930+
exists(boolean preservesValue |
931+
Private::Steps::summaryLocalStep(a.asNode(), b.asNode(), preservesValue) and
932+
if preservesValue = true then value = "value" else value = "taint"
933+
)
934+
or
935+
exists(Content c |
936+
Private::Steps::summaryReadStep(a.asNode(), c, b.asNode()) and
937+
value = "read (" + c + ")"
938+
or
939+
Private::Steps::summaryStoreStep(a.asNode(), c, b.asNode()) and
940+
value = "store (" + c + ")"
941+
or
942+
Private::Steps::summaryClearsContent(a.asNode(), c) and
943+
b = a and
944+
value = "clear (" + c + ")"
945+
)
946+
or
947+
summaryPostUpdateNode(b.asNode(), a.asNode()) and
948+
value = "post-update"
949+
or
950+
b.asCall() = summaryDataFlowCall(a.asNode()) and
951+
value = "receiver"
952+
or
953+
exists(int i |
954+
summaryArgumentNode(b.asCall(), a.asNode(), i) and
955+
value = "argument (" + i + ")"
956+
)
957+
}
958+
959+
query predicate edges(NodeOrCall a, NodeOrCall b, string key, string value) {
960+
key = "semmle.label" and
961+
value = strictconcat(string s | edgesComponent(a, b, s) | s, " / ")
962+
}
963+
}
873964
}

‎java/ql/test/library-tests/dataflow/callback-dispatch/A.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,4 +169,25 @@ void foo2() {
169169
sink(res2); // $ flow=19
170170
}
171171

172+
static void applyConsumer1Field1Field2(A a1, A a2, Consumer1 con) {
173+
// summary:
174+
// con.eat(a1.field1);
175+
// con.eat(a2.field2);
176+
}
177+
178+
static void wrapSinkToAvoidFieldSsa(A a) { sink(a.field1); }
179+
180+
void foo3() {
181+
A a1 = new A();
182+
a1.field1 = source(20);
183+
A a2 = new A();
184+
applyConsumer1Field1Field2(a1, a2, p -> {
185+
sink(p); // MISSING FLOW
186+
});
187+
wrapSinkToAvoidFieldSsa(a1); // MISSING FLOW
188+
sink(a2.field2);
189+
}
190+
191+
public Object field1;
192+
public Object field2;
172193
}

‎java/ql/test/library-tests/dataflow/callback-dispatch/test.ql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ class SummaryModelTest extends SummaryModelCsv {
88
row =
99
[
1010
"my.callback.qltest;A;false;applyConsumer1;(Object,Consumer1);;Argument[0];Parameter[0] of Argument[1];value",
11+
"my.callback.qltest;A;false;applyConsumer1Field1Field2;(A,A,Consumer1);;Field[my.callback.qltest.A.field1] of Argument[0];Parameter[0] of Argument[2];value",
12+
"my.callback.qltest;A;false;applyConsumer1Field1Field2;(A,A,Consumer1);;Field[my.callback.qltest.A.field2] of Argument[1];Parameter[0] of Argument[2];value",
1113
"my.callback.qltest;A;false;applyConsumer2;(Object,Consumer2);;Argument[0];Parameter[0] of Argument[1];value",
1214
"my.callback.qltest;A;false;applyConsumer3;(Object,Consumer3);;Argument[0];Parameter[0] of Argument[1];value",
1315
"my.callback.qltest;A;false;applyConsumer3_ret_postup;(Consumer3);;Parameter[0] of Argument[0];ReturnValue;value",

0 commit comments

Comments
 (0)