Merge pull request #4353 from hvitved/csharp/dataflow/task-precise

C#: Precise data-flow for `System.Threading.Tasks`

Author: hvitved

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

@@ -11,6 +11,7 @@ private import semmle.code.csharp.frameworks.system.io.Compression
 private import semmle.code.csharp.frameworks.system.linq.Expressions
 private import semmle.code.csharp.frameworks.system.Net
 private import semmle.code.csharp.frameworks.system.Text
+private import semmle.code.csharp.frameworks.system.runtime.CompilerServices
 private import semmle.code.csharp.frameworks.system.threading.Tasks
 private import semmle.code.csharp.frameworks.system.Web
 private import semmle.code.csharp.frameworks.system.web.ui.WebControls
@@ -34,6 +35,8 @@ private newtype TAccessPath =
     or
     tail = AccessPath::singleton(_) and
     head instanceof ElementContent
+    or
+    tail = AccessPath::element()
   }
 
 /** An access path. */
@@ -93,6 +96,11 @@ module AccessPath {
     result = singleton(any(PropertyContent c | c.getProperty() = p.getSourceDeclaration()))
   }
 
+  /** Gets a singleton field access path. */
+  AccessPath field(Field f) {
+    result = singleton(any(FieldContent c | c.getField() = f.getSourceDeclaration()))
+  }
+
   /** Gets an access path representing a property inside a collection. */
   AccessPath properties(Property p) { result = TConsAccessPath(any(ElementContent c), property(p)) }
 }
@@ -1665,7 +1673,6 @@ class SystemThreadingTasksTaskFlow extends LibraryTypeDataFlow, SystemThreadingT
     (
       m.hasName("ContinueWith") and
       sourceAp = AccessPath::empty() and
-      sinkAp = AccessPath::empty() and
       (
         // flow from supplied state to supplied delegate
         exists(ConstructedDelegateType delegate, int i, int j, int k |
@@ -1677,79 +1684,83 @@ class SystemThreadingTasksTaskFlow extends LibraryTypeDataFlow, SystemThreadingT
           ) and
           delegate.getTypeArgument(k) instanceof ObjectType and
           source = TCallableFlowSourceArg(i) and
-          sink = getDelegateFlowSinkArg(m, j, k)
+          sink = getDelegateFlowSinkArg(m, j, k) and
+          sinkAp = AccessPath::empty()
         )
         or
         // flow out of supplied function
         exists(ConstructedDelegateType func, int i |
           m.getParameter(i).getType() = func and
           func.getUnboundGeneric() instanceof SystemFuncDelegateType and
           source = getDelegateFlowSourceArg(m, i) and
-          sink = TCallableFlowSinkReturn()
+          sink = TCallableFlowSinkReturn() and
+          sinkAp = AccessPath::property(any(SystemThreadingTasksTaskTClass c).getResultProperty())
         )
       )
       or
       m.hasName("FromResult") and
+      source = TCallableFlowSourceArg(0) and
       sourceAp = AccessPath::empty() and
-      sinkAp = AccessPath::empty() and
-      (
-        source = TCallableFlowSourceArg(0) and
-        sink = TCallableFlowSinkReturn()
-      )
+      sink = TCallableFlowSinkReturn() and
+      sinkAp = AccessPath::property(any(SystemThreadingTasksTaskTClass c).getResultProperty())
       or
       m.hasName("Run") and
+      m.getReturnType() = any(SystemThreadingTasksTaskTClass c).getAConstructedGeneric() and
+      m.(UnboundGenericMethod).getNumberOfTypeParameters() = 1 and
+      source = TCallableFlowSourceDelegateArg(0) and
       sourceAp = AccessPath::empty() and
-      sinkAp = AccessPath::empty() and
-      (
-        m.getReturnType() = any(SystemThreadingTasksTaskTClass c).getAConstructedGeneric() and
-        m.(UnboundGenericMethod).getNumberOfTypeParameters() = 1 and
-        source = TCallableFlowSourceDelegateArg(0) and
-        sink = TCallableFlowSinkReturn()
-      )
+      sink = TCallableFlowSinkReturn() and
+      sinkAp = AccessPath::property(any(SystemThreadingTasksTaskTClass c).getResultProperty())
       or
       m.getName().regexpMatch("WhenAll|WhenAny") and
-      sinkAp = AccessPath::empty() and
-      (
-        m.getReturnType() = any(SystemThreadingTasksTaskTClass c).getAConstructedGeneric() and
-        m.(UnboundGenericMethod).getNumberOfTypeParameters() = 1 and
-        source = getFlowSourceArg(m, _, sourceAp) and
-        sink = TCallableFlowSinkReturn()
-      )
+      m.getReturnType() = any(SystemThreadingTasksTaskTClass c).getAConstructedGeneric() and
+      m.(UnboundGenericMethod).getNumberOfTypeParameters() = 1 and
+      source = getFlowSourceArg(m, _, _) and
+      sourceAp = AccessPath::properties(any(SystemThreadingTasksTaskTClass c).getResultProperty()) and
+      sink = TCallableFlowSinkReturn() and
+      sinkAp =
+        AccessPath::cons(any(PropertyContent c |
+            c.getProperty() = any(SystemThreadingTasksTaskTClass tc).getResultProperty()
+          ), AccessPath::element())
     )
   }
 }
 
 /** Data flow for `System.Threading.Tasks.Task<>`. */
-class SystemThreadingTasksTaskTFlow extends LibraryTypeDataFlow {
-  SystemThreadingTasksTaskTFlow() { this instanceof SystemThreadingTasksTaskTClass }
-
+class SystemThreadingTasksTaskTFlow extends LibraryTypeDataFlow, SystemThreadingTasksTaskTClass {
   override predicate callableFlow(
-    CallableFlowSource source, CallableFlowSink sink, SourceDeclarationCallable c,
-    boolean preservesValue
+    CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
+    SourceDeclarationCallable c, boolean preservesValue
   ) {
     (
-      constructorFlow(source, sink, c)
+      constructorFlow(source, sourceAp, sink, sinkAp, c)
       or
-      methodFlow(source, sink, c)
-      or
-      exists(Property p |
-        propertyFlow(p) and
-        source = TCallableFlowSourceQualifier() and
-        sink = TCallableFlowSinkReturn() and
-        c = p.getGetter()
-      )
+      methodFlow(source, sourceAp, sink, sinkAp, c)
     ) and
     preservesValue = true
+    or
+    exists(Property p |
+      p = this.(SystemThreadingTasksTaskTClass).getResultProperty() and
+      source = TCallableFlowSourceQualifier() and
+      sourceAp = AccessPath::empty() and
+      sink = TCallableFlowSinkReturn() and
+      sinkAp = AccessPath::empty() and
+      c = p.getGetter() and
+      preservesValue = false
+    )
   }
 
-  private predicate constructorFlow(CallableFlowSource source, CallableFlowSink sink, Constructor c) {
+  private predicate constructorFlow(
+    CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
+    Constructor c
+  ) {
     // flow from supplied function into constructed Task
     c.getDeclaringType() = this and
-    (
-      c.getParameter(0).getType() = any(SystemFuncDelegateType t).getAConstructedGeneric() and
-      source = TCallableFlowSourceDelegateArg(0) and
-      sink = TCallableFlowSinkReturn()
-    )
+    c.getParameter(0).getType() = any(SystemFuncDelegateType t).getAConstructedGeneric() and
+    source = TCallableFlowSourceDelegateArg(0) and
+    sourceAp = AccessPath::empty() and
+    sink = TCallableFlowSinkReturn() and
+    sinkAp = AccessPath::property(this.(SystemThreadingTasksTaskTClass).getResultProperty())
     or
     // flow from supplied state to supplied delegate
     c.getDeclaringType() = this and
@@ -1759,12 +1770,15 @@ class SystemThreadingTasksTaskTFlow extends LibraryTypeDataFlow {
       func.getUnboundGeneric().(SystemFuncDelegateType).getNumberOfTypeParameters() = 2 and
       func.getTypeArgument(0) instanceof ObjectType and
       source = TCallableFlowSourceArg(1) and
-      sink = getDelegateFlowSinkArg(c, 0, 0)
+      sourceAp = AccessPath::empty() and
+      sink = getDelegateFlowSinkArg(c, 0, 0) and
+      sinkAp = AccessPath::empty()
     )
   }
 
   private predicate methodFlow(
-    CallableFlowSource source, CallableFlowSink sink, SourceDeclarationMethod m
+    CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
+    SourceDeclarationMethod m
   ) {
     m.getDeclaringType() = this and
     m.hasName("ContinueWith") and
@@ -1781,27 +1795,37 @@ class SystemThreadingTasksTaskTFlow extends LibraryTypeDataFlow {
           delegate.getTypeArgument(j) instanceof ObjectType and
           m.getParameter(k).getType() instanceof ObjectType and
           source = TCallableFlowSourceArg(k) and
-          sink = getDelegateFlowSinkArg(m, i, j)
+          sourceAp = AccessPath::empty() and
+          sink = getDelegateFlowSinkArg(m, i, j) and
+          sinkAp = AccessPath::empty()
         )
         or
         // flow from this task to supplied delegate
         delegate.getTypeArgument(j) = this and
         source = TCallableFlowSourceQualifier() and
-        sink = getDelegateFlowSinkArg(m, i, j)
+        sourceAp = AccessPath::empty() and
+        sink = getDelegateFlowSinkArg(m, i, j) and
+        sinkAp = AccessPath::empty()
       )
       or
       // flow out of supplied function
       exists(ConstructedDelegateType func, int i |
         m.getParameter(i).getType() = func and
         func.getUnboundGeneric() instanceof SystemFuncDelegateType and
         source = getDelegateFlowSourceArg(m, i) and
-        sink = TCallableFlowSinkReturn()
+        sourceAp = AccessPath::empty() and
+        sink = TCallableFlowSinkReturn() and
+        sinkAp = AccessPath::property(this.(SystemThreadingTasksTaskTClass).getResultProperty())
       )
     )
-  }
-
-  private predicate propertyFlow(Property p) {
-    p = this.(SystemThreadingTasksTaskTClass).getResultProperty()
+    or
+    m = this.getGetAwaiterMethod() and
+    source = TCallableFlowSourceQualifier() and
+    sourceAp = AccessPath::empty() and
+    sink = TCallableFlowSinkReturn() and
+    sinkAp =
+      AccessPath::field(any(SystemRuntimeCompilerServicesTaskAwaiterStruct s)
+            .getUnderlyingTaskField())
   }
 }
 
@@ -1813,15 +1837,16 @@ class SystemThreadingTasksFactoryFlow extends LibraryTypeDataFlow {
   }
 
   override predicate callableFlow(
-    CallableFlowSource source, CallableFlowSink sink, SourceDeclarationCallable c,
-    boolean preservesValue
+    CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
+    SourceDeclarationCallable c, boolean preservesValue
   ) {
-    methodFlow(source, sink, c) and
+    methodFlow(source, sourceAp, sink, sinkAp, c) and
     preservesValue = true
   }
 
   private predicate methodFlow(
-    CallableFlowSource source, CallableFlowSink sink, SourceDeclarationMethod m
+    CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
+    SourceDeclarationMethod m
   ) {
     m.getDeclaringType() = this and
     (
@@ -1838,15 +1863,19 @@ class SystemThreadingTasksFactoryFlow extends LibraryTypeDataFlow {
             delegate.getUnboundGeneric() instanceof SystemFuncDelegateType
           ) and
           source = TCallableFlowSourceArg(i) and
-          sink = getDelegateFlowSinkArg(m, j, k)
+          sourceAp = AccessPath::empty() and
+          sink = getDelegateFlowSinkArg(m, j, k) and
+          sinkAp = AccessPath::empty()
         )
         or
         // flow out of supplied function
         exists(ConstructedDelegateType func, int i |
           m.getParameter(i).getType() = func and
           func.getUnboundGeneric() instanceof SystemFuncDelegateType and
           source = getDelegateFlowSourceArg(m, i) and
-          sink = TCallableFlowSinkReturn()
+          sourceAp = AccessPath::empty() and
+          sink = TCallableFlowSinkReturn() and
+          sinkAp = AccessPath::property(any(SystemThreadingTasksTaskTClass c).getResultProperty())
         )
       )
       or
@@ -1862,21 +1891,48 @@ class SystemThreadingTasksFactoryFlow extends LibraryTypeDataFlow {
           ) and
           delegate.getTypeArgument(k) instanceof ObjectType and
           source = TCallableFlowSourceArg(i) and
-          sink = getDelegateFlowSinkArg(m, j, k)
+          sourceAp = AccessPath::empty() and
+          sink = getDelegateFlowSinkArg(m, j, k) and
+          sinkAp = AccessPath::empty()
         )
         or
         // flow out of supplied function
         exists(ConstructedDelegateType func, int i |
           m.getParameter(i).getType() = func and
           func.getUnboundGeneric() instanceof SystemFuncDelegateType and
           source = getDelegateFlowSourceArg(m, i) and
-          sink = TCallableFlowSinkReturn()
+          sourceAp = AccessPath::empty() and
+          sink = TCallableFlowSinkReturn() and
+          sinkAp = AccessPath::property(any(SystemThreadingTasksTaskTClass c).getResultProperty())
         )
       )
     )
   }
 }
 
+/** Data flow for `System.Runtime.CompilerServices.TaskAwaiter<>`. */
+class SystemRuntimeCompilerServicesTaskAwaiterFlow extends LibraryTypeDataFlow,
+  SystemRuntimeCompilerServicesTaskAwaiterStruct {
+  override predicate callableFlow(
+    CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
+    SourceDeclarationCallable c, boolean preservesValue
+  ) {
+    preservesValue = true and
+    c = this.getGetResultMethod() and
+    source = TCallableFlowSourceQualifier() and
+    sourceAp =
+      AccessPath::cons(any(FieldContent fc | fc.getField() = this.getUnderlyingTaskField()),
+        AccessPath::property(any(SystemThreadingTasksTaskTClass t).getResultProperty())) and
+    sink = TCallableFlowSinkReturn() and
+    sinkAp = AccessPath::empty()
+  }
+
+  override predicate requiresAccessPath(Content head, AccessPath tail) {
+    head.(FieldContent).getField() = this.getUnderlyingTaskField() and
+    tail = AccessPath::property(any(SystemThreadingTasksTaskTClass t).getResultProperty())
+  }
+}
+
 /** Data flow for `System.Text.Encoding`. */
 library class SystemTextEncodingFlow extends LibraryTypeDataFlow, SystemTextEncodingClass {
   override predicate callableFlow(

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

@@ -16,6 +16,7 @@ private import semmle.code.csharp.dispatch.Dispatch
 private import semmle.code.csharp.frameworks.EntityFramework
 private import semmle.code.csharp.frameworks.NHibernate
 private import semmle.code.csharp.frameworks.system.Collections
+private import semmle.code.csharp.frameworks.system.threading.Tasks
 
 abstract class NodeImpl extends Node {
   /** Do not call: use `getEnclosingCallable()` instead. */
@@ -154,10 +155,6 @@ module LocalFlow {
         scope = e2 and
         isSuccessor = true
         or
-        e1 = e2.(AwaitExpr).getExpr() and
-        scope = e2 and
-        isSuccessor = true
-        or
         // An `=` expression, where the result of the expression is used
         e2 =
           any(AssignExpr ae |
@@ -679,6 +676,11 @@ private module Cached {
     storeStepLibrary(node1, c, node2)
   }
 
+  pragma[nomagic]
+  private PropertyContent getResultContent() {
+    result.getProperty() = any(SystemThreadingTasksTaskTClass c_).getResultProperty()
+  }
+
   /**
    * Holds if data can flow from `node1` to `node2` via a read of content `c`.
    */
@@ -699,6 +701,10 @@ private module Cached {
         node2.(SsaDefinitionNode).getDefinition() = def and
         c instanceof ElementContent
       )
+      or
+      x.hasNodePath(node1, node2) and
+      node2.asExpr().(AwaitExpr).getExpr() = node1.asExpr() and
+      c = getResultContent()
     )
     or
     readStepLibrary(node1, c, node2)
@@ -2180,6 +2186,11 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration
     isSuccessor = true and
     arrayRead(e1, e2) and
     scope = e2
+    or
+    exactScope = false and
+    e1 = e2.(AwaitExpr).getExpr() and
+    scope = e2 and
+    isSuccessor = true
   }
 
   override predicate candidateDef(

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

@@ -95,6 +95,10 @@ private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityCon
           scope = e2 and
           isSuccessor = true
         )
+      or
+      e1 = e2.(AwaitExpr).getExpr() and
+      scope = e2 and
+      isSuccessor = true
     )
   }
 }