Skip to content

Commit 732ef92

Browse files
tamasvajktamasvajk
committed
C#: add store step for return statements inside async methods
1 parent c684b74 commit 732ef92

File tree

3 files changed

+75
-5
lines changed

3 files changed

+75
-5
lines changed

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,9 @@ private module Cached {
632632
TYieldReturnNode(ControlFlow::Nodes::ElementNode cfn) {
633633
any(Callable c).canYieldReturn(cfn.getElement())
634634
} or
635+
TAsyncReturnNode(ControlFlow::Nodes::ElementNode cfn) {
636+
any(Callable c | c.(Modifiable).isAsync()).canReturn(cfn.getElement())
637+
} or
635638
TImplicitCapturedArgumentNode(ControlFlow::Nodes::ElementNode cfn, LocalScopeVariable v) {
636639
exists(Ssa::ExplicitDefinition def | def.isCapturedVariableDefinitionFlowIn(_, cfn, _) |
637640
v = def.getSourceVariable().getAssignable()
@@ -783,6 +786,12 @@ private module Cached {
783786
c instanceof ElementContent
784787
)
785788
or
789+
exists(Expr e |
790+
e = node1.asExpr() and
791+
node2.(AsyncReturnNode).getReturnStmt().getExpr() = e and
792+
c = getResultContent()
793+
)
794+
or
786795
FlowSummaryImpl::Private::storeStep(node1, c, node2)
787796
}
788797

@@ -962,6 +971,8 @@ private module Cached {
962971
or
963972
n instanceof YieldReturnNode
964973
or
974+
n instanceof AsyncReturnNode
975+
or
965976
n instanceof ImplicitCapturedArgumentNode
966977
or
967978
n instanceof MallocNode
@@ -1397,6 +1408,30 @@ private module ReturnNodes {
13971408
override string toStringImpl() { result = yrs.toString() }
13981409
}
13991410

1411+
/**
1412+
* A synthesized `return` node for returned expressions inside `async` methods.
1413+
*/
1414+
class AsyncReturnNode extends ReturnNode, NodeImpl, TAsyncReturnNode {
1415+
private ControlFlow::Nodes::ElementNode cfn;
1416+
private ReturnStmt rs;
1417+
1418+
AsyncReturnNode() { this = TAsyncReturnNode(cfn) and rs.getExpr().getAControlFlowNode() = cfn }
1419+
1420+
ReturnStmt getReturnStmt() { result = rs }
1421+
1422+
override NormalReturnKind getKind() { any() }
1423+
1424+
override Callable getEnclosingCallableImpl() { result = rs.getEnclosingCallable() }
1425+
1426+
override Type getTypeImpl() { result = rs.getEnclosingCallable().getReturnType() }
1427+
1428+
override ControlFlow::Node getControlFlowNodeImpl() { result = cfn }
1429+
1430+
override Location getLocationImpl() { result = rs.getLocation() }
1431+
1432+
override string toStringImpl() { result = rs.toString() }
1433+
}
1434+
14001435
/**
14011436
* The value of a captured variable as an implicit return from a call, viewed
14021437
* as a node in a data flow graph.

‎csharp/ql/test/library-tests/dataflow/async/Async.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ private void Sink(string s)
88

99
public void TestNonAwait(string input)
1010
{
11-
Sink(Return(input)); // True positive
11+
Sink(Return(input));
1212
}
1313

1414
private string Return(string x)
@@ -18,18 +18,18 @@ private string Return(string x)
1818

1919
public async Task TestAwait1(string input)
2020
{
21-
Sink(await ReturnAwait(input)); // False negative
21+
Sink(await ReturnAwait(input));
2222
}
2323

2424
public async Task TestAwait2(string input)
2525
{
2626
var x = await ReturnAwait(input);
27-
Sink(x); // False negative
27+
Sink(x);
2828
}
2929

3030
public void TestAwait3(string input)
3131
{
32-
Sink(ReturnAwait(input).Result); // False negative
32+
Sink(ReturnAwait(input).Result);
3333
}
3434

3535
private async Task<string> ReturnAwait(string x)
@@ -40,7 +40,7 @@ private async Task<string> ReturnAwait(string x)
4040

4141
public void TestTask(string input)
4242
{
43-
Sink(ReturnTask(input).Result); // True positive
43+
Sink(ReturnTask(input).Result);
4444
}
4545

4646
private Task<string> ReturnTask(string x)

‎csharp/ql/test/library-tests/dataflow/async/Async.expected

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,20 @@ edges
33
| Async.cs:11:21:11:25 | access to parameter input : String | Async.cs:11:14:11:26 | call to method Return |
44
| Async.cs:14:34:14:34 | x : String | Async.cs:16:16:16:16 | access to parameter x : String |
55
| Async.cs:16:16:16:16 | access to parameter x : String | Async.cs:11:14:11:26 | call to method Return |
6+
| Async.cs:19:41:19:45 | input : String | Async.cs:21:32:21:36 | access to parameter input : String |
7+
| Async.cs:21:20:21:37 | call to method ReturnAwait [Result] : String | Async.cs:21:14:21:37 | await ... |
8+
| Async.cs:21:32:21:36 | access to parameter input : String | Async.cs:21:20:21:37 | call to method ReturnAwait [Result] : String |
9+
| Async.cs:24:41:24:45 | input : String | Async.cs:26:35:26:39 | access to parameter input : String |
10+
| Async.cs:26:17:26:40 | await ... : String | Async.cs:27:14:27:14 | access to local variable x |
11+
| Async.cs:26:23:26:40 | call to method ReturnAwait [Result] : String | Async.cs:26:17:26:40 | await ... : String |
12+
| Async.cs:26:35:26:39 | access to parameter input : String | Async.cs:26:23:26:40 | call to method ReturnAwait [Result] : String |
13+
| Async.cs:30:35:30:39 | input : String | Async.cs:32:26:32:30 | access to parameter input : String |
14+
| Async.cs:32:14:32:31 | call to method ReturnAwait [Result] : String | Async.cs:32:14:32:38 | access to property Result |
15+
| Async.cs:32:26:32:30 | access to parameter input : String | Async.cs:32:14:32:31 | call to method ReturnAwait [Result] : String |
16+
| Async.cs:35:51:35:51 | x : String | Async.cs:38:16:38:16 | access to parameter x : String |
17+
| Async.cs:38:16:38:16 | access to parameter x : String | Async.cs:21:20:21:37 | call to method ReturnAwait [Result] : String |
18+
| Async.cs:38:16:38:16 | access to parameter x : String | Async.cs:26:23:26:40 | call to method ReturnAwait [Result] : String |
19+
| Async.cs:38:16:38:16 | access to parameter x : String | Async.cs:32:14:32:31 | call to method ReturnAwait [Result] : String |
620
| Async.cs:41:33:41:37 | input : String | Async.cs:43:25:43:29 | access to parameter input : String |
721
| Async.cs:43:14:43:30 | call to method ReturnTask [Result] : String | Async.cs:43:14:43:37 | access to property Result |
822
| Async.cs:43:25:43:29 | access to parameter input : String | Async.cs:43:14:43:30 | call to method ReturnTask [Result] : String |
@@ -15,6 +29,21 @@ nodes
1529
| Async.cs:11:21:11:25 | access to parameter input : String | semmle.label | access to parameter input : String |
1630
| Async.cs:14:34:14:34 | x : String | semmle.label | x : String |
1731
| Async.cs:16:16:16:16 | access to parameter x : String | semmle.label | access to parameter x : String |
32+
| Async.cs:19:41:19:45 | input : String | semmle.label | input : String |
33+
| Async.cs:21:14:21:37 | await ... | semmle.label | await ... |
34+
| Async.cs:21:20:21:37 | call to method ReturnAwait [Result] : String | semmle.label | call to method ReturnAwait [Result] : String |
35+
| Async.cs:21:32:21:36 | access to parameter input : String | semmle.label | access to parameter input : String |
36+
| Async.cs:24:41:24:45 | input : String | semmle.label | input : String |
37+
| Async.cs:26:17:26:40 | await ... : String | semmle.label | await ... : String |
38+
| Async.cs:26:23:26:40 | call to method ReturnAwait [Result] : String | semmle.label | call to method ReturnAwait [Result] : String |
39+
| Async.cs:26:35:26:39 | access to parameter input : String | semmle.label | access to parameter input : String |
40+
| Async.cs:27:14:27:14 | access to local variable x | semmle.label | access to local variable x |
41+
| Async.cs:30:35:30:39 | input : String | semmle.label | input : String |
42+
| Async.cs:32:14:32:31 | call to method ReturnAwait [Result] : String | semmle.label | call to method ReturnAwait [Result] : String |
43+
| Async.cs:32:14:32:38 | access to property Result | semmle.label | access to property Result |
44+
| Async.cs:32:26:32:30 | access to parameter input : String | semmle.label | access to parameter input : String |
45+
| Async.cs:35:51:35:51 | x : String | semmle.label | x : String |
46+
| Async.cs:38:16:38:16 | access to parameter x : String | semmle.label | access to parameter x : String |
1847
| Async.cs:41:33:41:37 | input : String | semmle.label | input : String |
1948
| Async.cs:43:14:43:30 | call to method ReturnTask [Result] : String | semmle.label | call to method ReturnTask [Result] : String |
2049
| Async.cs:43:14:43:37 | access to property Result | semmle.label | access to property Result |
@@ -25,5 +54,11 @@ nodes
2554
#select
2655
| Async.cs:11:14:11:26 | call to method Return | Async.cs:9:37:9:41 | input : String | Async.cs:11:14:11:26 | call to method Return | $@ flows to here and is used. | Async.cs:9:37:9:41 | input | User-provided value |
2756
| Async.cs:11:14:11:26 | call to method Return | Async.cs:14:34:14:34 | x : String | Async.cs:11:14:11:26 | call to method Return | $@ flows to here and is used. | Async.cs:14:34:14:34 | x | User-provided value |
57+
| Async.cs:21:14:21:37 | await ... | Async.cs:19:41:19:45 | input : String | Async.cs:21:14:21:37 | await ... | $@ flows to here and is used. | Async.cs:19:41:19:45 | input | User-provided value |
58+
| Async.cs:21:14:21:37 | await ... | Async.cs:35:51:35:51 | x : String | Async.cs:21:14:21:37 | await ... | $@ flows to here and is used. | Async.cs:35:51:35:51 | x | User-provided value |
59+
| Async.cs:27:14:27:14 | access to local variable x | Async.cs:24:41:24:45 | input : String | Async.cs:27:14:27:14 | access to local variable x | $@ flows to here and is used. | Async.cs:24:41:24:45 | input | User-provided value |
60+
| Async.cs:27:14:27:14 | access to local variable x | Async.cs:35:51:35:51 | x : String | Async.cs:27:14:27:14 | access to local variable x | $@ flows to here and is used. | Async.cs:35:51:35:51 | x | User-provided value |
61+
| Async.cs:32:14:32:38 | access to property Result | Async.cs:30:35:30:39 | input : String | Async.cs:32:14:32:38 | access to property Result | $@ flows to here and is used. | Async.cs:30:35:30:39 | input | User-provided value |
62+
| Async.cs:32:14:32:38 | access to property Result | Async.cs:35:51:35:51 | x : String | Async.cs:32:14:32:38 | access to property Result | $@ flows to here and is used. | Async.cs:35:51:35:51 | x | User-provided value |
2863
| Async.cs:43:14:43:37 | access to property Result | Async.cs:41:33:41:37 | input : String | Async.cs:43:14:43:37 | access to property Result | $@ flows to here and is used. | Async.cs:41:33:41:37 | input | User-provided value |
2964
| Async.cs:43:14:43:37 | access to property Result | Async.cs:46:44:46:44 | x : String | Async.cs:43:14:43:37 | access to property Result | $@ flows to here and is used. | Async.cs:46:44:46:44 | x | User-provided value |

0 commit comments

Comments
 (0)