Skip to content

Commit 3c26779

Browse files
hvitvedhvitved
authored
Merge pull request github#5415 from tamasvajk/feature/async-flow
C#: add store step for return statements inside async methods
2 parents 27408fe + 0b1705f commit 3c26779

File tree

7 files changed

+195
-48
lines changed

7 files changed

+195
-48
lines changed

‎csharp/ql/src/semmle/code/csharp/Callable.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,11 @@ class Callable extends DotNet::Callable, Parameterizable, ExprOrStmtParent, @cal
206206
exists(ReturnStmt ret | ret.getEnclosingCallable() = this | e = ret.getExpr())
207207
or
208208
e = this.getExpressionBody() and
209-
not this.getReturnType() instanceof VoidType
209+
not this.getReturnType() instanceof VoidType and
210+
(
211+
not this.(Modifiable).isAsync() or
212+
this.getReturnType() instanceof Generic
213+
)
210214
}
211215

212216
/** Holds if this callable can yield return the expression `e`. */

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,6 @@ private module Cached {
8282
cached
8383
newtype TReturnKind =
8484
TNormalReturnKind() { Stages::DataFlowStage::forceCachingInSameStage() } or
85-
TYieldReturnKind() or
8685
TOutReturnKind(int i) {
8786
exists(Parameter p | callableReturnsOutOrRef(_, p, _) and p.isOut() | i = p.getPosition())
8887
} or
@@ -179,11 +178,6 @@ class NormalReturnKind extends ReturnKind, TNormalReturnKind {
179178
override string toString() { result = "return" }
180179
}
181180

182-
/** A value returned from a callable using a `yield return` statement. */
183-
class YieldReturnKind extends ReturnKind, TYieldReturnKind {
184-
override string toString() { result = "yield return" }
185-
}
186-
187181
/** A value returned from a callable using an `out` or a `ref` parameter. */
188182
abstract class OutRefReturnKind extends ReturnKind {
189183
/** Gets the position of the `out`/`ref` parameter. */

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

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,9 @@ private module Cached {
631631
TYieldReturnNode(ControlFlow::Nodes::ElementNode cfn) {
632632
any(Callable c).canYieldReturn(cfn.getElement())
633633
} or
634+
TAsyncReturnNode(ControlFlow::Nodes::ElementNode cfn) {
635+
any(Callable c | c.(Modifiable).isAsync()).canReturn(cfn.getElement())
636+
} or
634637
TImplicitCapturedArgumentNode(ControlFlow::Nodes::ElementNode cfn, LocalScopeVariable v) {
635638
exists(Ssa::ExplicitDefinition def | def.isCapturedVariableDefinitionFlowIn(_, cfn, _) |
636639
v = def.getSourceVariable().getAssignable()
@@ -782,6 +785,12 @@ private module Cached {
782785
c instanceof ElementContent
783786
)
784787
or
788+
exists(Expr e |
789+
e = node1.asExpr() and
790+
node2.(AsyncReturnNode).getExpr() = e and
791+
c = getResultContent()
792+
)
793+
or
785794
FlowSummaryImpl::Private::storeStep(node1, c, node2)
786795
}
787796

@@ -961,6 +970,8 @@ private module Cached {
961970
or
962971
n instanceof YieldReturnNode
963972
or
973+
n instanceof AsyncReturnNode
974+
or
964975
n instanceof ImplicitCapturedArgumentNode
965976
or
966977
n instanceof MallocNode
@@ -1342,14 +1353,12 @@ abstract class ReturnNode extends Node {
13421353
private module ReturnNodes {
13431354
/**
13441355
* A data-flow node that represents an expression returned by a callable,
1345-
* either using a (`yield`) `return` statement or an expression body (`=>`).
1356+
* either using a `return` statement or an expression body (`=>`).
13461357
*/
13471358
class ExprReturnNode extends ReturnNode, ExprNode {
13481359
ExprReturnNode() {
13491360
exists(DotNet::Callable c, DotNet::Expr e | e = this.getExpr() |
1350-
c.canReturn(e)
1351-
or
1352-
c.(Callable).canYieldReturn(e)
1361+
c.canReturn(e) and not c.(Modifiable).isAsync()
13531362
)
13541363
}
13551364

@@ -1383,7 +1392,7 @@ private module ReturnNodes {
13831392

13841393
YieldReturnStmt getYieldReturnStmt() { result = yrs }
13851394

1386-
override YieldReturnKind getKind() { any() }
1395+
override NormalReturnKind getKind() { any() }
13871396

13881397
override Callable getEnclosingCallableImpl() { result = yrs.getEnclosingCallable() }
13891398

@@ -1396,6 +1405,30 @@ private module ReturnNodes {
13961405
override string toStringImpl() { result = yrs.toString() }
13971406
}
13981407

1408+
/**
1409+
* A synthesized `return` node for returned expressions inside `async` methods.
1410+
*/
1411+
class AsyncReturnNode extends ReturnNode, NodeImpl, TAsyncReturnNode {
1412+
private ControlFlow::Nodes::ElementNode cfn;
1413+
private Expr expr;
1414+
1415+
AsyncReturnNode() { this = TAsyncReturnNode(cfn) and expr = cfn.getElement() }
1416+
1417+
Expr getExpr() { result = expr }
1418+
1419+
override NormalReturnKind getKind() { any() }
1420+
1421+
override Callable getEnclosingCallableImpl() { result = expr.getEnclosingCallable() }
1422+
1423+
override Type getTypeImpl() { result = expr.getEnclosingCallable().getReturnType() }
1424+
1425+
override ControlFlow::Node getControlFlowNodeImpl() { result = cfn }
1426+
1427+
override Location getLocationImpl() { result = expr.getLocation() }
1428+
1429+
override string toStringImpl() { result = expr.toString() }
1430+
}
1431+
13991432
/**
14001433
* The value of a captured variable as an implicit return from a call, viewed
14011434
* as a node in a data flow graph.
@@ -1482,21 +1515,6 @@ private module OutNodes {
14821515
result = TExplicitDelegateLikeCall(cfn, e)
14831516
}
14841517

1485-
/** A valid return type for a method that uses `yield return`. */
1486-
private class YieldReturnType extends Type {
1487-
YieldReturnType() {
1488-
exists(Type t | t = this.getUnboundDeclaration() |
1489-
t instanceof SystemCollectionsIEnumerableInterface
1490-
or
1491-
t instanceof SystemCollectionsIEnumeratorInterface
1492-
or
1493-
t instanceof SystemCollectionsGenericIEnumerableTInterface
1494-
or
1495-
t instanceof SystemCollectionsGenericIEnumeratorInterface
1496-
)
1497-
}
1498-
}
1499-
15001518
/**
15011519
* A data-flow node that reads a value returned directly by a callable,
15021520
* either via a C# call or a CIL call.
@@ -1517,9 +1535,6 @@ private module OutNodes {
15171535
(
15181536
kind instanceof NormalReturnKind and
15191537
not call.getExpr().getType() instanceof VoidType
1520-
or
1521-
kind instanceof YieldReturnKind and
1522-
call.getExpr().getType() instanceof YieldReturnType
15231538
)
15241539
}
15251540
}

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
using System.Threading.Tasks;
2+
3+
class Test
4+
{
5+
private void Sink(string s)
6+
{
7+
}
8+
9+
public void TestNonAwait(string input)
10+
{
11+
Sink(Return(input));
12+
}
13+
14+
private string Return(string x)
15+
{
16+
return x;
17+
}
18+
19+
public async Task TestAwait1(string input)
20+
{
21+
Sink(await ReturnAwait(input));
22+
}
23+
24+
public async Task TestAwait2(string input)
25+
{
26+
var x = await ReturnAwait(input);
27+
Sink(x);
28+
}
29+
30+
public void TestAwait3(string input)
31+
{
32+
Sink(ReturnAwait2(input).Result);
33+
}
34+
35+
private async Task<string> ReturnAwait(string x)
36+
{
37+
await Task.Delay(1);
38+
return x;
39+
}
40+
41+
public void TestTask(string input)
42+
{
43+
Sink(ReturnTask(input).Result);
44+
}
45+
46+
private Task<string> ReturnTask(string x)
47+
{
48+
return Task.FromResult(x);
49+
}
50+
51+
private async Task<string> ReturnAwait2(string x) => x;
52+
}

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
edges
2+
| Async.cs:9:37:9:41 | input : String | Async.cs:11:21:11:25 | access to parameter input : String |
3+
| Async.cs:11:21:11:25 | access to parameter input : String | Async.cs:11:14:11:26 | call to method Return |
4+
| Async.cs:14:34:14:34 | x : String | Async.cs:16:16:16:16 | access to parameter x : String |
5+
| 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:27:32:31 | access to parameter input : String |
14+
| Async.cs:32:14:32:32 | call to method ReturnAwait2 [Result] : String | Async.cs:32:14:32:39 | access to property Result |
15+
| Async.cs:32:27:32:31 | access to parameter input : String | Async.cs:32:14:32:32 | call to method ReturnAwait2 [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:41:33:41:37 | input : String | Async.cs:43:25:43:29 | access to parameter input : String |
20+
| Async.cs:43:14:43:30 | call to method ReturnTask [Result] : String | Async.cs:43:14:43:37 | access to property Result |
21+
| Async.cs:43:25:43:29 | access to parameter input : String | Async.cs:43:14:43:30 | call to method ReturnTask [Result] : String |
22+
| Async.cs:46:44:46:44 | x : String | Async.cs:48:32:48:32 | access to parameter x : String |
23+
| Async.cs:48:16:48:33 | call to method FromResult [Result] : String | Async.cs:43:14:43:30 | call to method ReturnTask [Result] : String |
24+
| Async.cs:48:32:48:32 | access to parameter x : String | Async.cs:48:16:48:33 | call to method FromResult [Result] : String |
25+
| Async.cs:51:52:51:52 | x : String | Async.cs:51:58:51:58 | access to parameter x : String |
26+
| Async.cs:51:58:51:58 | access to parameter x : String | Async.cs:32:14:32:32 | call to method ReturnAwait2 [Result] : String |
27+
nodes
28+
| Async.cs:9:37:9:41 | input : String | semmle.label | input : String |
29+
| Async.cs:11:14:11:26 | call to method Return | semmle.label | call to method Return |
30+
| Async.cs:11:21:11:25 | access to parameter input : String | semmle.label | access to parameter input : String |
31+
| Async.cs:14:34:14:34 | x : String | semmle.label | x : String |
32+
| Async.cs:16:16:16:16 | access to parameter x : String | semmle.label | access to parameter x : String |
33+
| Async.cs:19:41:19:45 | input : String | semmle.label | input : String |
34+
| Async.cs:21:14:21:37 | await ... | semmle.label | await ... |
35+
| Async.cs:21:20:21:37 | call to method ReturnAwait [Result] : String | semmle.label | call to method ReturnAwait [Result] : String |
36+
| Async.cs:21:32:21:36 | access to parameter input : String | semmle.label | access to parameter input : String |
37+
| Async.cs:24:41:24:45 | input : String | semmle.label | input : String |
38+
| Async.cs:26:17:26:40 | await ... : String | semmle.label | await ... : String |
39+
| Async.cs:26:23:26:40 | call to method ReturnAwait [Result] : String | semmle.label | call to method ReturnAwait [Result] : String |
40+
| Async.cs:26:35:26:39 | access to parameter input : String | semmle.label | access to parameter input : String |
41+
| Async.cs:27:14:27:14 | access to local variable x | semmle.label | access to local variable x |
42+
| Async.cs:30:35:30:39 | input : String | semmle.label | input : String |
43+
| Async.cs:32:14:32:32 | call to method ReturnAwait2 [Result] : String | semmle.label | call to method ReturnAwait2 [Result] : String |
44+
| Async.cs:32:14:32:39 | access to property Result | semmle.label | access to property Result |
45+
| Async.cs:32:27:32:31 | access to parameter input : String | semmle.label | access to parameter input : String |
46+
| Async.cs:35:51:35:51 | x : String | semmle.label | x : String |
47+
| Async.cs:38:16:38:16 | access to parameter x : String | semmle.label | access to parameter x : String |
48+
| Async.cs:41:33:41:37 | input : String | semmle.label | input : String |
49+
| Async.cs:43:14:43:30 | call to method ReturnTask [Result] : String | semmle.label | call to method ReturnTask [Result] : String |
50+
| Async.cs:43:14:43:37 | access to property Result | semmle.label | access to property Result |
51+
| Async.cs:43:25:43:29 | access to parameter input : String | semmle.label | access to parameter input : String |
52+
| Async.cs:46:44:46:44 | x : String | semmle.label | x : String |
53+
| Async.cs:48:16:48:33 | call to method FromResult [Result] : String | semmle.label | call to method FromResult [Result] : String |
54+
| Async.cs:48:32:48:32 | access to parameter x : String | semmle.label | access to parameter x : String |
55+
| Async.cs:51:52:51:52 | x : String | semmle.label | x : String |
56+
| Async.cs:51:58:51:58 | access to parameter x : String | semmle.label | access to parameter x : String |
57+
#select
58+
| 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 |
59+
| 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 |
60+
| 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 |
61+
| 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 |
62+
| 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 |
63+
| 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 |
64+
| Async.cs:32:14:32:39 | access to property Result | Async.cs:30:35:30:39 | input : String | Async.cs:32:14:32:39 | access to property Result | $@ flows to here and is used. | Async.cs:30:35:30:39 | input | User-provided value |
65+
| Async.cs:32:14:32:39 | access to property Result | Async.cs:51:52:51:52 | x : String | Async.cs:32:14:32:39 | access to property Result | $@ flows to here and is used. | Async.cs:51:52:51:52 | x | User-provided value |
66+
| 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 |
67+
| 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 |

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import csharp
2+
import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
3+
4+
class MySink extends DataFlow::ExprNode {
5+
MySink() {
6+
exists(Method m, MethodCall mc |
7+
mc.getTarget() = m and
8+
m.getName() = "Sink" and
9+
this.getExpr() = mc.getArgumentForName("s")
10+
)
11+
}
12+
}
13+
14+
class MySource extends DataFlow::ParameterNode {
15+
MySource() {
16+
exists(Parameter p | p = this.getParameter() |
17+
p = any(Class c | c.hasQualifiedName("Test")).getAMethod().getAParameter()
18+
)
19+
}
20+
}
21+
22+
class MyConfig extends TaintTracking::Configuration {
23+
MyConfig() { this = "MyConfig" }
24+
25+
override predicate isSource(DataFlow::Node source) { source instanceof MySource }
26+
27+
override predicate isSink(DataFlow::Node sink) { sink instanceof MySink }
28+
}
29+
30+
from MyConfig c, DataFlow::PathNode source, DataFlow::PathNode sink
31+
where c.hasFlowPath(source, sink)
32+
select sink.getNode(), source, sink, "$@ flows to here and is used.", source.getNode(),
33+
"User-provided value"

0 commit comments

Comments
 (0)