C#: Add another group of assignable definitions (corresponding to compound assignment operations) and update taint flow for +.

Author: michaelnebel

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

@@ -78,6 +78,8 @@ class AssignableRead extends AssignableAccess {
       this.isRefArgument()
       or
       this = any(AssignableDefinitions::AddressOfDefinition def).getTargetAccess()
+      or
+      this = any(AssignableDefinitions::AssignOperationDefinition def).getTargetAccess()
     ) and
     not nameOfChild(_, this)
   }
@@ -271,6 +273,8 @@ module AssignableInternal {
     def = TAddressOfDefinition(result)
     or
     def = TPatternDefinition(result)
+    or
+    def = TAssignOperationDefinition(result)
   }
 
   /** A local variable declaration at the top-level of a pattern. */
@@ -286,7 +290,10 @@ module AssignableInternal {
   private module Cached {
     cached
     newtype TAssignableDefinition =
-      TAssignmentDefinition(Assignment a) { not a.getLValue() instanceof TupleExpr } or
+      TAssignmentDefinition(Assignment a) {
+        not a.getLValue() instanceof TupleExpr and
+        not a instanceof AssignOperation
+      } or
       TTupleAssignmentDefinition(AssignExpr ae, Expr leaf) { tupleAssignmentDefinition(ae, leaf) } or
       TOutRefDefinition(AssignableAccess aa) {
         aa.isOutArgument()
@@ -309,7 +316,8 @@ module AssignableInternal {
         )
       } or
       TAddressOfDefinition(AddressOfExpr aoe) or
-      TPatternDefinition(TopLevelPatternDecl tlpd)
+      TPatternDefinition(TopLevelPatternDecl tlpd) or
+      TAssignOperationDefinition(AssignOperation ao)
 
     /**
      * Gets the source expression assigned in tuple definition `def`, if any.
@@ -355,6 +363,8 @@ module AssignableInternal {
       def = TMutationDefinition(any(MutatorOperation mo | mo.getOperand() = result))
       or
       def = TAddressOfDefinition(any(AddressOfExpr aoe | aoe.getOperand() = result))
+      or
+      def = TAssignOperationDefinition(any(AssignOperation ao | ao.getLeftOperand() = result))
     }
 
     /**
@@ -388,8 +398,9 @@ private import AssignableInternal
  * a mutation update (`AssignableDefinitions::MutationDefinition`), a local variable
  * declaration without an initializer (`AssignableDefinitions::LocalVariableDefinition`),
  * an implicit parameter definition (`AssignableDefinitions::ImplicitParameterDefinition`),
- * an address-of definition (`AssignableDefinitions::AddressOfDefinition`), or a pattern
- * definition (`AssignableDefinitions::PatternDefinition`).
+ * an address-of definition (`AssignableDefinitions::AddressOfDefinition`), a pattern
+ * definition (`AssignableDefinitions::PatternDefinition`), or a compound assignment
+ * operation definition (`AssignableDefinitions::AssignOperationDefinition`)
  */
 class AssignableDefinition extends TAssignableDefinition {
   /**
@@ -511,7 +522,7 @@ module AssignableDefinitions {
 
     override Expr getSource() {
       result = a.getRValue() and
-      not a instanceof AssignOperation
+      not a instanceof AddOrRemoveEventExpr
     }
 
     override string toString() { result = a.toString() }
@@ -735,4 +746,17 @@ module AssignableDefinitions {
     /** Gets the assignable (field or property) being initialized. */
     Assignable getAssignable() { result = fieldOrProp }
   }
+
+  /**
+   * A definition by a compound assignment operation, for example `x += y`.
+   */
+  class AssignOperationDefinition extends AssignableDefinition, TAssignOperationDefinition {
+    AssignOperation ao;
+
+    AssignOperationDefinition() { this = TAssignOperationDefinition(ao) }
+
+    override Expr getSource() { result = ao }
+
+    override string toString() { result = ao.toString() }
+  }
 }

csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll

@@ -486,6 +486,7 @@ module Expressions {
         result = qe.getChild(i)
       )
     or
+    // TODO: This can be fixed if the extracted indexes are fixed.
     e =
       any(Assignment a |
         // The left-hand side of an assignment is evaluated before the right-hand side

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

@@ -49,6 +49,8 @@ private predicate localTaintExprStep(Expr e1, Expr e2) {
   or
   e1 = e2.(AddExpr).getAnOperand()
   or
+  e1 = e2.(AssignAddExpr).getAnOperand()
+  or
   // A comparison expression where taint can flow from one of the
   // operands if the other operand is a constant value.
   exists(ComparisonTest ct, Expr other |