C#: Recognize ref assignments through delegate calls

Author: hvitved

csharp/ql/src/semmle/code/csharp/Assignable.qll

@@ -162,9 +162,8 @@ module AssignableInternal {
    * Holds if the `ref` assignment to `aa` via call `c` is relevant.
    */
   private predicate isRelevantRefCall(Call c, AssignableAccess aa) {
-    c.getAnArgument() = aa and
-    aa.isRefArgument() and
-    (isNonAnalyzableRefCall(c, aa, _) or exists(getAnAnalyzableRefDef(c, aa, _)))
+    isNonAnalyzableRefCall(c, aa) or
+    exists(getAnAnalyzableRefDef(c, aa, _))
   }
 
   private Callable getRefCallTarget(Call c, AssignableAccess aa, Parameter p) {
@@ -220,10 +219,16 @@ module AssignableInternal {
    * Equivalent with `not isAnalyzableRefCall(mc, aa, p)`, but avoids negative
    * recursion.
    */
-  private predicate isNonAnalyzableRefCall(Call c, AssignableAccess aa, Parameter p) {
-    exists(Callable callable | callable = getRefCallTarget(c, aa, p) |
-      callable.(Virtualizable).isOverridableOrImplementable() or
-      not callable.hasBody()
+  private predicate isNonAnalyzableRefCall(Call c, AssignableAccess aa) {
+    aa = c.getAnArgument() and
+    aa.isRefArgument() and
+    (
+      not exists(getRefCallTarget(c, aa, _))
+      or
+      exists(Callable callable | callable = getRefCallTarget(c, aa, _) |
+        callable.(Virtualizable).isOverridableOrImplementable() or
+        not callable.hasBody()
+      )
     )
   }
 
@@ -275,7 +280,8 @@ module AssignableInternal {
         not lvde.hasInitializer() and
         not exists(getTupleSource(TTupleAssignmentDefinition(_, lvde))) and
         not lvde = any(IsPatternExpr ipe).getVariableDeclExpr() and
-        not lvde = any(TypeCase tc).getVariableDeclExpr()
+        not lvde = any(TypeCase tc).getVariableDeclExpr() and
+        not lvde.isOutArgument()
       } or
       TImplicitParameterDefinition(Parameter p) {
         exists(Callable c | p = c.getAParameter() |

csharp/ql/test/library-tests/assignables/AssignableAccess.expected

@@ -92,6 +92,7 @@
 | Assignables.cs:123:13:123:13 | access to local variable x | Assignables.cs:123:13:123:13 | x | write |
 | Assignables.cs:124:9:124:9 | access to parameter d | Assignables.cs:121:31:121:31 | d | read |
 | Assignables.cs:124:15:124:15 | access to local variable x | Assignables.cs:123:13:123:13 | x | read |
+| Assignables.cs:124:15:124:15 | access to local variable x | Assignables.cs:123:13:123:13 | x | write |
 | Assignables.cs:124:29:124:29 | String s | Assignables.cs:124:29:124:29 | s | write |
 | Discards.cs:7:9:7:9 | access to parameter x | Discards.cs:5:30:5:30 | x | write |
 | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | z | write |

csharp/ql/test/library-tests/assignables/AssignableDefinition.expected

@@ -80,15 +80,14 @@
 | Assignables.cs:116:13:116:13 | s | Assignables.cs:117:9:117:30 | ... = ... | Assignables.cs:117:9:117:9 | access to local variable s | Assignables.cs:117:13:117:30 | nameof(...) | certain |
 | Assignables.cs:121:31:121:31 | d | Assignables.cs:121:31:121:31 | d | Assignables.cs:121:31:121:31 | <none> | Assignables.cs:121:31:121:31 | <none> | certain |
 | Assignables.cs:123:13:123:13 | x | Assignables.cs:123:13:123:17 | Int32 x = ... | Assignables.cs:123:13:123:13 | access to local variable x | Assignables.cs:123:17:123:17 | 0 | certain |
-| Assignables.cs:124:29:124:29 | s | Assignables.cs:124:29:124:29 | String s | Assignables.cs:124:29:124:29 | <none> | Assignables.cs:124:29:124:29 | <none> | certain |
+| Assignables.cs:123:13:123:13 | x | Assignables.cs:124:15:124:15 | access to local variable x | Assignables.cs:124:15:124:15 | access to local variable x | Assignables.cs:124:15:124:15 | <none> | certain |
 | Assignables.cs:124:29:124:29 | s | Assignables.cs:124:29:124:29 | String s | Assignables.cs:124:29:124:29 | String s | Assignables.cs:124:29:124:29 | <none> | certain |
 | Discards.cs:5:30:5:30 | x | Discards.cs:5:30:5:30 | x | Discards.cs:5:30:5:30 | <none> | Discards.cs:5:30:5:30 | <none> | certain |
 | Discards.cs:5:30:5:30 | x | Discards.cs:7:9:7:17 | ... = ... | Discards.cs:7:9:7:9 | access to parameter x | Discards.cs:7:13:7:17 | false | certain |
 | Discards.cs:19:14:19:14 | x | Discards.cs:19:9:19:29 | ... = ... | Discards.cs:19:9:19:29 | <none> | Discards.cs:19:9:19:29 | <none> | certain |
 | Discards.cs:19:14:19:14 | x | Discards.cs:19:14:19:14 | Int32 x | Discards.cs:19:14:19:14 | <none> | Discards.cs:19:14:19:14 | <none> | certain |
 | Discards.cs:20:17:20:17 | y | Discards.cs:20:9:20:33 | ... = ... | Discards.cs:20:9:20:33 | <none> | Discards.cs:20:9:20:33 | <none> | certain |
 | Discards.cs:20:17:20:17 | y | Discards.cs:20:17:20:17 | Double y | Discards.cs:20:17:20:17 | <none> | Discards.cs:20:17:20:17 | <none> | certain |
-| Discards.cs:20:32:20:32 | z | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | <none> | Discards.cs:20:32:20:32 | <none> | certain |
 | Discards.cs:20:32:20:32 | z | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | <none> | certain |
 | Discards.cs:23:27:23:30 | args | Discards.cs:23:27:23:30 | args | Discards.cs:23:27:23:30 | <none> | Discards.cs:23:27:23:30 | <none> | certain |
 | Discards.cs:30:24:30:24 | o | Discards.cs:30:24:30:24 | o | Discards.cs:30:24:30:24 | <none> | Discards.cs:30:24:30:24 | <none> | certain |

csharp/ql/test/library-tests/assignables/AssignableDefinitionNode.expected

@@ -79,8 +79,8 @@
 | Assignables.cs:117:9:117:30 | ... = ... | Assignables.cs:117:9:117:30 | ... = ... |
 | Assignables.cs:121:31:121:31 | d | Assignables.cs:121:10:121:20 | enter DelegateRef |
 | Assignables.cs:123:13:123:17 | Int32 x = ... | Assignables.cs:123:13:123:17 | Int32 x = ... |
+| Assignables.cs:124:15:124:15 | access to local variable x | Assignables.cs:124:9:124:30 | delegate call |
 | Assignables.cs:124:29:124:29 | String s | Assignables.cs:124:9:124:30 | delegate call |
-| Assignables.cs:124:29:124:29 | String s | Assignables.cs:124:29:124:29 | String s |
 | Discards.cs:5:30:5:30 | x | Discards.cs:5:19:5:19 | enter f |
 | Discards.cs:7:9:7:17 | ... = ... | Discards.cs:7:9:7:17 | ... = ... |
 | Discards.cs:13:9:13:20 | ... = ... | Discards.cs:13:9:13:20 | ... = ... |
@@ -93,7 +93,6 @@
 | Discards.cs:20:9:20:33 | ... = ... | Discards.cs:20:9:20:33 | ... = ... |
 | Discards.cs:20:17:20:17 | Double y | Discards.cs:20:17:20:17 | Double y |
 | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:22:20:33 | call to method f |
-| Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | Boolean z |
 | Discards.cs:23:27:23:30 | args | Discards.cs:23:10:23:16 | enter Foreach |
 | Discards.cs:30:24:30:24 | o | Discards.cs:30:10:30:15 | enter Switch |
 | Finally.cs:5:16:5:16 | b | Finally.cs:5:9:5:9 | enter M |