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))
     }
 
     /**
@@ -509,10 +519,7 @@ module AssignableDefinitions {
     /** Gets the underlying assignment. */
     Assignment getAssignment() { result = a }
 
-    override Expr getSource() {
-      result = a.getRValue() and
-      not a instanceof AssignOperation
-    }
+    override Expr getSource() { result = a.getRValue() }
 
     override string toString() { result = a.toString() }
   }
@@ -735,4 +742,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

@@ -60,6 +60,9 @@ private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityCon
       e1 = e2.(AddExpr).getAnOperand() and
       scope = e2
       or
+      e1 = e2.(AssignAddExpr).getAnOperand() and
+      scope = e2
+      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 |