Skip to content

Commit c0b251a

Browse files
hvitvedhvitved
committed
C#: Precise data-flow for System.Threading.Tasks 
1 parent 26544f3 commit c0b251a

File tree

6 files changed

+206
-150
lines changed

6 files changed

+206
-150
lines changed

‎csharp/ql/src/semmle/code/csharp/dataflow/LibraryTypeDataFlow.qll‎

Lines changed: 77 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ private newtype TAccessPath =
3434
or
3535
tail = AccessPath::singleton(_) and
3636
head instanceof ElementContent
37+
or
38+
tail = AccessPath::element()
3739
}
3840

3941
/** An access path. */
@@ -1658,7 +1660,6 @@ class SystemThreadingTasksTaskFlow extends LibraryTypeDataFlow, SystemThreadingT
16581660
(
16591661
m.hasName("ContinueWith") and
16601662
sourceAp = AccessPath::empty() and
1661-
sinkAp = AccessPath::empty() and
16621663
(
16631664
// flow from supplied state to supplied delegate
16641665
exists(ConstructedDelegateType delegate, int i, int j, int k |
@@ -1670,44 +1671,44 @@ class SystemThreadingTasksTaskFlow extends LibraryTypeDataFlow, SystemThreadingT
16701671
) and
16711672
delegate.getTypeArgument(k) instanceof ObjectType and
16721673
source = TCallableFlowSourceArg(i) and
1673-
sink = getDelegateFlowSinkArg(m, j, k)
1674+
sink = getDelegateFlowSinkArg(m, j, k) and
1675+
sinkAp = AccessPath::empty()
16741676
)
16751677
or
16761678
// flow out of supplied function
16771679
exists(ConstructedDelegateType func, int i |
16781680
m.getParameter(i).getType() = func and
16791681
func.getUnboundGeneric() instanceof SystemFuncDelegateType and
16801682
source = getDelegateFlowSourceArg(m, i) and
1681-
sink = TCallableFlowSinkReturn()
1683+
sink = TCallableFlowSinkReturn() and
1684+
sinkAp = AccessPath::property(any(SystemThreadingTasksTaskTClass c).getResultProperty())
16821685
)
16831686
)
16841687
or
16851688
m.hasName("FromResult") and
1689+
source = TCallableFlowSourceArg(0) and
16861690
sourceAp = AccessPath::empty() and
1687-
sinkAp = AccessPath::empty() and
1688-
(
1689-
source = TCallableFlowSourceArg(0) and
1690-
sink = TCallableFlowSinkReturn()
1691-
)
1691+
sink = TCallableFlowSinkReturn() and
1692+
sinkAp = AccessPath::property(any(SystemThreadingTasksTaskTClass c).getResultProperty())
16921693
or
16931694
m.hasName("Run") and
1695+
m.getReturnType() = any(SystemThreadingTasksTaskTClass c).getAConstructedGeneric() and
1696+
m.(UnboundGenericMethod).getNumberOfTypeParameters() = 1 and
1697+
source = TCallableFlowSourceDelegateArg(0) and
16941698
sourceAp = AccessPath::empty() and
1695-
sinkAp = AccessPath::empty() and
1696-
(
1697-
m.getReturnType() = any(SystemThreadingTasksTaskTClass c).getAConstructedGeneric() and
1698-
m.(UnboundGenericMethod).getNumberOfTypeParameters() = 1 and
1699-
source = TCallableFlowSourceDelegateArg(0) and
1700-
sink = TCallableFlowSinkReturn()
1701-
)
1699+
sink = TCallableFlowSinkReturn() and
1700+
sinkAp = AccessPath::property(any(SystemThreadingTasksTaskTClass c).getResultProperty())
17021701
or
17031702
m.getName().regexpMatch("WhenAll|WhenAny") and
1704-
sinkAp = AccessPath::empty() and
1705-
(
1706-
m.getReturnType() = any(SystemThreadingTasksTaskTClass c).getAConstructedGeneric() and
1707-
m.(UnboundGenericMethod).getNumberOfTypeParameters() = 1 and
1708-
source = getFlowSourceArg(m, _, sourceAp) and
1709-
sink = TCallableFlowSinkReturn()
1710-
)
1703+
m.getReturnType() = any(SystemThreadingTasksTaskTClass c).getAConstructedGeneric() and
1704+
m.(UnboundGenericMethod).getNumberOfTypeParameters() = 1 and
1705+
source = getFlowSourceArg(m, _, _) and
1706+
sourceAp = AccessPath::properties(any(SystemThreadingTasksTaskTClass c).getResultProperty()) and
1707+
sink = TCallableFlowSinkReturn() and
1708+
sinkAp =
1709+
AccessPath::cons(any(PropertyContent c |
1710+
c.getProperty() = any(SystemThreadingTasksTaskTClass tc).getResultProperty()
1711+
), AccessPath::element())
17111712
)
17121713
}
17131714
}
@@ -1717,32 +1718,38 @@ class SystemThreadingTasksTaskTFlow extends LibraryTypeDataFlow {
17171718
SystemThreadingTasksTaskTFlow() { this instanceof SystemThreadingTasksTaskTClass }
17181719

17191720
override predicate callableFlow(
1720-
CallableFlowSource source, CallableFlowSink sink, SourceDeclarationCallable c,
1721-
boolean preservesValue
1721+
CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
1722+
SourceDeclarationCallable c, boolean preservesValue
17221723
) {
17231724
(
1724-
constructorFlow(source, sink, c)
1725+
constructorFlow(source, sourceAp, sink, sinkAp, c)
17251726
or
1726-
methodFlow(source, sink, c)
1727-
or
1728-
exists(Property p |
1729-
propertyFlow(p) and
1730-
source = TCallableFlowSourceQualifier() and
1731-
sink = TCallableFlowSinkReturn() and
1732-
c = p.getGetter()
1733-
)
1727+
methodFlow(source, sourceAp, sink, sinkAp, c)
17341728
) and
17351729
preservesValue = true
1730+
or
1731+
exists(Property p |
1732+
p = this.(SystemThreadingTasksTaskTClass).getResultProperty() and
1733+
source = TCallableFlowSourceQualifier() and
1734+
sourceAp = AccessPath::empty() and
1735+
sink = TCallableFlowSinkReturn() and
1736+
sinkAp = AccessPath::empty() and
1737+
c = p.getGetter() and
1738+
preservesValue = false
1739+
)
17361740
}
17371741

1738-
private predicate constructorFlow(CallableFlowSource source, CallableFlowSink sink, Constructor c) {
1742+
private predicate constructorFlow(
1743+
CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
1744+
Constructor c
1745+
) {
17391746
// flow from supplied function into constructed Task
17401747
c.getDeclaringType() = this and
1741-
(
1742-
c.getParameter(0).getType() = any(SystemFuncDelegateType t).getAConstructedGeneric() and
1743-
source = TCallableFlowSourceDelegateArg(0) and
1744-
sink = TCallableFlowSinkReturn()
1745-
)
1748+
c.getParameter(0).getType() = any(SystemFuncDelegateType t).getAConstructedGeneric() and
1749+
source = TCallableFlowSourceDelegateArg(0) and
1750+
sourceAp = AccessPath::empty() and
1751+
sink = TCallableFlowSinkReturn() and
1752+
sinkAp = AccessPath::property(this.(SystemThreadingTasksTaskTClass).getResultProperty())
17461753
or
17471754
// flow from supplied state to supplied delegate
17481755
c.getDeclaringType() = this and
@@ -1752,12 +1759,15 @@ class SystemThreadingTasksTaskTFlow extends LibraryTypeDataFlow {
17521759
func.getUnboundGeneric().(SystemFuncDelegateType).getNumberOfTypeParameters() = 2 and
17531760
func.getTypeArgument(0) instanceof ObjectType and
17541761
source = TCallableFlowSourceArg(1) and
1755-
sink = getDelegateFlowSinkArg(c, 0, 0)
1762+
sourceAp = AccessPath::empty() and
1763+
sink = getDelegateFlowSinkArg(c, 0, 0) and
1764+
sinkAp = AccessPath::empty()
17561765
)
17571766
}
17581767

17591768
private predicate methodFlow(
1760-
CallableFlowSource source, CallableFlowSink sink, SourceDeclarationMethod m
1769+
CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
1770+
SourceDeclarationMethod m
17611771
) {
17621772
m.getDeclaringType() = this and
17631773
m.hasName("ContinueWith") and
@@ -1774,28 +1784,30 @@ class SystemThreadingTasksTaskTFlow extends LibraryTypeDataFlow {
17741784
delegate.getTypeArgument(j) instanceof ObjectType and
17751785
m.getParameter(k).getType() instanceof ObjectType and
17761786
source = TCallableFlowSourceArg(k) and
1777-
sink = getDelegateFlowSinkArg(m, i, j)
1787+
sourceAp = AccessPath::empty() and
1788+
sink = getDelegateFlowSinkArg(m, i, j) and
1789+
sinkAp = AccessPath::empty()
17781790
)
17791791
or
17801792
// flow from this task to supplied delegate
17811793
delegate.getTypeArgument(j) = this and
17821794
source = TCallableFlowSourceQualifier() and
1783-
sink = getDelegateFlowSinkArg(m, i, j)
1795+
sourceAp = AccessPath::empty() and
1796+
sink = getDelegateFlowSinkArg(m, i, j) and
1797+
sinkAp = AccessPath::empty()
17841798
)
17851799
or
17861800
// flow out of supplied function
17871801
exists(ConstructedDelegateType func, int i |
17881802
m.getParameter(i).getType() = func and
17891803
func.getUnboundGeneric() instanceof SystemFuncDelegateType and
17901804
source = getDelegateFlowSourceArg(m, i) and
1791-
sink = TCallableFlowSinkReturn()
1805+
sourceAp = AccessPath::empty() and
1806+
sink = TCallableFlowSinkReturn() and
1807+
sinkAp = AccessPath::property(this.(SystemThreadingTasksTaskTClass).getResultProperty())
17921808
)
17931809
)
17941810
}
1795-
1796-
private predicate propertyFlow(Property p) {
1797-
p = this.(SystemThreadingTasksTaskTClass).getResultProperty()
1798-
}
17991811
}
18001812

18011813
/** Data flow for `System.Threading.Tasks.TaskFactory`(`<TResult>`). */
@@ -1806,15 +1818,16 @@ class SystemThreadingTasksFactoryFlow extends LibraryTypeDataFlow {
18061818
}
18071819

18081820
override predicate callableFlow(
1809-
CallableFlowSource source, CallableFlowSink sink, SourceDeclarationCallable c,
1810-
boolean preservesValue
1821+
CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
1822+
SourceDeclarationCallable c, boolean preservesValue
18111823
) {
1812-
methodFlow(source, sink, c) and
1824+
methodFlow(source, sourceAp, sink, sinkAp, c) and
18131825
preservesValue = true
18141826
}
18151827

18161828
private predicate methodFlow(
1817-
CallableFlowSource source, CallableFlowSink sink, SourceDeclarationMethod m
1829+
CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
1830+
SourceDeclarationMethod m
18181831
) {
18191832
m.getDeclaringType() = this and
18201833
(
@@ -1831,15 +1844,19 @@ class SystemThreadingTasksFactoryFlow extends LibraryTypeDataFlow {
18311844
delegate.getUnboundGeneric() instanceof SystemFuncDelegateType
18321845
) and
18331846
source = TCallableFlowSourceArg(i) and
1834-
sink = getDelegateFlowSinkArg(m, j, k)
1847+
sourceAp = AccessPath::empty() and
1848+
sink = getDelegateFlowSinkArg(m, j, k) and
1849+
sinkAp = AccessPath::empty()
18351850
)
18361851
or
18371852
// flow out of supplied function
18381853
exists(ConstructedDelegateType func, int i |
18391854
m.getParameter(i).getType() = func and
18401855
func.getUnboundGeneric() instanceof SystemFuncDelegateType and
18411856
source = getDelegateFlowSourceArg(m, i) and
1842-
sink = TCallableFlowSinkReturn()
1857+
sourceAp = AccessPath::empty() and
1858+
sink = TCallableFlowSinkReturn() and
1859+
sinkAp = AccessPath::property(any(SystemThreadingTasksTaskTClass c).getResultProperty())
18431860
)
18441861
)
18451862
or
@@ -1855,15 +1872,19 @@ class SystemThreadingTasksFactoryFlow extends LibraryTypeDataFlow {
18551872
) and
18561873
delegate.getTypeArgument(k) instanceof ObjectType and
18571874
source = TCallableFlowSourceArg(i) and
1858-
sink = getDelegateFlowSinkArg(m, j, k)
1875+
sourceAp = AccessPath::empty() and
1876+
sink = getDelegateFlowSinkArg(m, j, k) and
1877+
sinkAp = AccessPath::empty()
18591878
)
18601879
or
18611880
// flow out of supplied function
18621881
exists(ConstructedDelegateType func, int i |
18631882
m.getParameter(i).getType() = func and
18641883
func.getUnboundGeneric() instanceof SystemFuncDelegateType and
18651884
source = getDelegateFlowSourceArg(m, i) and
1866-
sink = TCallableFlowSinkReturn()
1885+
sourceAp = AccessPath::empty() and
1886+
sink = TCallableFlowSinkReturn() and
1887+
sinkAp = AccessPath::property(any(SystemThreadingTasksTaskTClass c).getResultProperty())
18671888
)
18681889
)
18691890
)

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ private import semmle.code.csharp.dispatch.Dispatch
1616
private import semmle.code.csharp.frameworks.EntityFramework
1717
private import semmle.code.csharp.frameworks.NHibernate
1818
private import semmle.code.csharp.frameworks.system.Collections
19+
private import semmle.code.csharp.frameworks.system.threading.Tasks
1920

2021
abstract class NodeImpl extends Node {
2122
/** Do not call: use `getEnclosingCallable()` instead. */
@@ -154,10 +155,6 @@ module LocalFlow {
154155
scope = e2 and
155156
isSuccessor = true
156157
or
157-
e1 = e2.(AwaitExpr).getExpr() and
158-
scope = e2 and
159-
isSuccessor = true
160-
or
161158
// An `=` expression, where the result of the expression is used
162159
e2 =
163160
any(AssignExpr ae |
@@ -679,6 +676,11 @@ private module Cached {
679676
storeStepLibrary(node1, c, node2)
680677
}
681678

679+
pragma[nomagic]
680+
private PropertyContent getResultContent() {
681+
result.getProperty() = any(SystemThreadingTasksTaskTClass c_).getResultProperty()
682+
}
683+
682684
/**
683685
* Holds if data can flow from `node1` to `node2` via a read of content `c`.
684686
*/
@@ -699,6 +701,10 @@ private module Cached {
699701
node2.(SsaDefinitionNode).getDefinition() = def and
700702
c instanceof ElementContent
701703
)
704+
or
705+
x.hasNodePath(node1, node2) and
706+
node2.asExpr().(AwaitExpr).getExpr() = node1.asExpr() and
707+
c = getResultContent()
702708
)
703709
or
704710
readStepLibrary(node1, c, node2)
@@ -2180,6 +2186,11 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration
21802186
isSuccessor = true and
21812187
arrayRead(e1, e2) and
21822188
scope = e2
2189+
or
2190+
exactScope = false and
2191+
e1 = e2.(AwaitExpr).getExpr() and
2192+
scope = e2 and
2193+
isSuccessor = true
21832194
}
21842195

21852196
override predicate candidateDef(

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityCon
9595
scope = e2 and
9696
isSuccessor = true
9797
)
98+
or
99+
e1 = e2.(AwaitExpr).getExpr() and
100+
scope = e2 and
101+
isSuccessor = true
98102
)
99103
}
100104
}

‎csharp/ql/test/library-tests/dataflow/global/DataFlowPath.expected‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,13 @@ edges
180180
| GlobalDataFlow.cs:217:22:217:28 | access to local variable tainted [[]] : String | GlobalDataFlow.cs:324:32:324:42 | sinkParam11 : String |
181181
| GlobalDataFlow.cs:217:22:217:49 | call to method Select [[]] : String | GlobalDataFlow.cs:217:22:217:57 | call to method First : String |
182182
| GlobalDataFlow.cs:217:22:217:57 | call to method First : String | GlobalDataFlow.cs:218:15:218:20 | access to local variable sink26 |
183-
| GlobalDataFlow.cs:238:35:238:48 | "taint source" : String | GlobalDataFlow.cs:240:15:240:20 | access to local variable sink41 |
184-
| GlobalDataFlow.cs:238:35:238:48 | "taint source" : String | GlobalDataFlow.cs:242:15:242:20 | access to local variable sink42 |
183+
| GlobalDataFlow.cs:238:20:238:49 | call to method Run [Result] : String | GlobalDataFlow.cs:239:22:239:25 | access to local variable task [Result] : String |
184+
| GlobalDataFlow.cs:238:20:238:49 | call to method Run [Result] : String | GlobalDataFlow.cs:241:28:241:31 | access to local variable task [Result] : String |
185+
| GlobalDataFlow.cs:238:35:238:48 | "taint source" : String | GlobalDataFlow.cs:238:20:238:49 | call to method Run [Result] : String |
186+
| GlobalDataFlow.cs:239:22:239:25 | access to local variable task [Result] : String | GlobalDataFlow.cs:239:22:239:32 | access to property Result : String |
187+
| GlobalDataFlow.cs:239:22:239:32 | access to property Result : String | GlobalDataFlow.cs:240:15:240:20 | access to local variable sink41 |
188+
| GlobalDataFlow.cs:241:22:241:31 | await ... : String | GlobalDataFlow.cs:242:15:242:20 | access to local variable sink42 |
189+
| GlobalDataFlow.cs:241:28:241:31 | access to local variable task [Result] : String | GlobalDataFlow.cs:241:22:241:31 | await ... : String |
185190
| GlobalDataFlow.cs:254:26:254:35 | sinkParam0 : String | GlobalDataFlow.cs:256:16:256:25 | access to parameter sinkParam0 : String |
186191
| GlobalDataFlow.cs:254:26:254:35 | sinkParam0 : String | GlobalDataFlow.cs:257:15:257:24 | access to parameter sinkParam0 |
187192
| GlobalDataFlow.cs:256:16:256:25 | access to parameter sinkParam0 : String | GlobalDataFlow.cs:254:26:254:35 | sinkParam0 : String |
@@ -367,8 +372,13 @@ nodes
367372
| GlobalDataFlow.cs:217:22:217:49 | call to method Select [[]] : String | semmle.label | call to method Select [[]] : String |
368373
| GlobalDataFlow.cs:217:22:217:57 | call to method First : String | semmle.label | call to method First : String |
369374
| GlobalDataFlow.cs:218:15:218:20 | access to local variable sink26 | semmle.label | access to local variable sink26 |
375+
| GlobalDataFlow.cs:238:20:238:49 | call to method Run [Result] : String | semmle.label | call to method Run [Result] : String |
370376
| GlobalDataFlow.cs:238:35:238:48 | "taint source" : String | semmle.label | "taint source" : String |
377+
| GlobalDataFlow.cs:239:22:239:25 | access to local variable task [Result] : String | semmle.label | access to local variable task [Result] : String |
378+
| GlobalDataFlow.cs:239:22:239:32 | access to property Result : String | semmle.label | access to property Result : String |
371379
| GlobalDataFlow.cs:240:15:240:20 | access to local variable sink41 | semmle.label | access to local variable sink41 |
380+
| GlobalDataFlow.cs:241:22:241:31 | await ... : String | semmle.label | await ... : String |
381+
| GlobalDataFlow.cs:241:28:241:31 | access to local variable task [Result] : String | semmle.label | access to local variable task [Result] : String |
372382
| GlobalDataFlow.cs:242:15:242:20 | access to local variable sink42 | semmle.label | access to local variable sink42 |
373383
| GlobalDataFlow.cs:254:26:254:35 | sinkParam0 : String | semmle.label | sinkParam0 : String |
374384
| GlobalDataFlow.cs:256:16:256:25 | access to parameter sinkParam0 : String | semmle.label | access to parameter sinkParam0 : String |

0 commit comments

Comments
 (0)