Merge pull request #437 from calumgrant/cs/in-parameters

C#: Correctly handle `in` arguments

change-notes/1.19/analysis-csharp.md

@@ -3,7 +3,7 @@
 ## General improvements
 
 * Control flow graph improvements:
-  * The control flow graph construction now takes simple Boolean conditions on local scope variables into account. For example, in `if (b) x = 0; if (b) x = 1;`, the control flow graph will reflect that taking the `true` (resp. `false`) branch in the first condition implies taking the same branch in the second condition. In effect, the first assignment to `x` will now be identified as being dead.
+* The control flow graph construction now takes simple Boolean conditions on local scope variables into account. For example, in `if (b) x = 0; if (b) x = 1;`, the control flow graph will reflect that taking the `true` (resp. `false`) branch in the first condition implies taking the same branch in the second condition. In effect, the first assignment to `x` will now be identified as being dead.
   * Code that is only reachable from a constant failing assertion, such as `Debug.Assert(false)`, is considered to be unreachable.
 
 ## New queries
@@ -20,7 +20,11 @@
 | Cross-site scripting (`cs/web/xss`) | More results | This query now finds cross-site scripting vulnerabilities in ASP.NET Core applications. |
 | *@name of query (Query ID)*| *Impact on results*    | *How/why the query has changed*                                  |
 
+## Changes to code extraction
+
+* Arguments passed using `in` are now extracted.
 
 ## Changes to QL libraries
 
 * `getArgument()` on `AccessorCall` has been improved so it now takes tuple assignments into account. For example, the argument for the implicit `value` parameter in the setter of property `P` is `0` in `(P, x) = (0, 1)`. Additionally, the argument for the `value` parameter in compound assignments is now only the expanded value, for example, in `P += 7` the argument is `P + 7` and not `7`.
+* The predicate `isInArgument()` has been added to the `AssignableAccess` class. This holds for expressions that are passed as arguments using `in`.

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expression.cs

@@ -223,6 +223,9 @@ namespace Semmle.Extraction.CSharp.Entities
                 case SyntaxKind.None:
                     mode = 0;
                     break;
+                case SyntaxKind.InKeyword:
+                    mode = 3;
+                    break;
                 default:
                     throw new InternalError(arg, "Unknown argument type");
             }

csharp/ql/src/semmle/code/csharp/exprs/Access.qll

@@ -174,6 +174,14 @@ class AssignableAccess extends Access, @assignable_access_expr {
     isOutArgument() or
     isRefArgument()
   }
+
+  /**
+   * Holds if this access passes the assignable being accessed as an `in`
+   * argument in a method call.
+   */
+  predicate isInArgument() {
+    expr_argument(this, 3)
+  }
 }
 
 /**

csharp/ql/test/library-tests/csharp7.2/InArguments.expected

@@ -0,0 +1 @@
+| csharp72.cs:18:12:18:12 | access to local variable s |

csharp/ql/test/library-tests/csharp7.2/InArguments.ql

@@ -0,0 +1,5 @@
+import csharp
+
+from AssignableAccess e
+where e.isInArgument()
+select e

csharp/ql/test/library-tests/csharp7.2/NumericLiterals.expected

@@ -1,2 +1,2 @@
-| csharp72.cs:42:23:42:34 | 85 |
-| csharp72.cs:47:31:47:31 | 1 |
+| csharp72.cs:48:23:48:34 | 85 |
+| csharp72.cs:53:31:53:31 | 1 |

csharp/ql/test/library-tests/csharp7.2/PrivateProtected.expected

@@ -1,2 +1,2 @@
-| csharp72.cs:47:27:47:27 | X |
-| csharp72.cs:49:28:49:28 | F |
+| csharp72.cs:53:27:53:27 | X |
+| csharp72.cs:55:28:55:28 | F |

csharp/ql/test/library-tests/csharp7.2/ReadonlyStructs.expected

@@ -1,2 +1,2 @@
-| csharp72.cs:28:17:28:30 | ReadonlyStruct |
-| csharp72.cs:36:21:36:37 | ReadonlyRefStruct |
+| csharp72.cs:34:17:34:30 | ReadonlyStruct |
+| csharp72.cs:42:21:42:37 | ReadonlyRefStruct |

csharp/ql/test/library-tests/csharp7.2/RefReadonlyDelegate.expected

@@ -1 +1 @@
-| csharp72.cs:25:31:25:33 | Del |
+| csharp72.cs:31:31:31:33 | Del |

csharp/ql/test/library-tests/csharp7.2/RefReadonlyReturns.expected

@@ -1 +1 @@
-| csharp72.cs:20:22:20:22 | F |
+| csharp72.cs:26:22:26:22 | F |

csharp/ql/test/library-tests/csharp7.2/RefStructs.expected

@@ -1,2 +1,2 @@
-| csharp72.cs:32:12:32:20 | RefStruct |
-| csharp72.cs:36:21:36:37 | ReadonlyRefStruct |
+| csharp72.cs:38:12:38:20 | RefStruct |
+| csharp72.cs:42:21:42:37 | ReadonlyRefStruct |

csharp/ql/test/library-tests/csharp7.2/csharp72.cs

@@ -11,6 +11,12 @@ class InModifiers
     void F(in S s)
     {
     }
+
+    void CallF()
+    {
+      var s = new S();
+      F(in s);
+    }
 }
 
 class RefReadonlyReturns