Merge pull request #15010 from tamasvajk/fix/stringbuilder-interpolation

C#: Support interpolated strings in `StringBuilder.Append`

csharp/ql/lib/change-notes/2023-12-07-stringbuilder.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+
+* The dataflow models for the `System.Text.StringBuilder` class have been reworked. New summaries have been added for `Append` and `AppendLine`. With the changes, we expect queries that use taint tracking to find more results when interpolated strings or `StringBuilder` instances are passed to `Append` or `AppendLine`.

csharp/ql/lib/ext/System.Text.model.yml

@@ -45,10 +45,16 @@ extensions:
       - ["System.Text", "StringBuilder", False, "Append", "(System.String,System.Int32,System.Int32)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
       - ["System.Text", "StringBuilder", False, "Append", "(System.String,System.Int32,System.Int32)", "", "Argument[this]", "ReturnValue", "value", "manual"]
       - ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder)", "", "Argument[this]", "ReturnValue", "value", "manual"]
+      - ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
       - ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder,System.Int32,System.Int32)", "", "Argument[this]", "ReturnValue", "value", "manual"]
+      - ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder,System.Int32,System.Int32)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
       - ["System.Text", "StringBuilder", False, "Append", "(System.UInt16)", "", "Argument[this]", "ReturnValue", "value", "manual"]
       - ["System.Text", "StringBuilder", False, "Append", "(System.UInt32)", "", "Argument[this]", "ReturnValue", "value", "manual"]
       - ["System.Text", "StringBuilder", False, "Append", "(System.UInt64)", "", "Argument[this]", "ReturnValue", "value", "manual"]
+      - ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[this]", "ReturnValue", "value", "manual"]
+      - ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
+      - ["System.Text", "StringBuilder", False, "Append", "(System.IFormatProvider,System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[this]", "ReturnValue", "value", "manual"]
+      - ["System.Text", "StringBuilder", False, "Append", "(System.IFormatProvider,System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[1]", "Argument[this]", "taint", "manual"]
       - ["System.Text", "StringBuilder", False, "AppendFormat", "(System.IFormatProvider,System.String,System.Object)", "", "Argument[1]", "Argument[this]", "taint", "manual"]
       - ["System.Text", "StringBuilder", False, "AppendFormat", "(System.IFormatProvider,System.String,System.Object)", "", "Argument[2]", "Argument[this]", "taint", "manual"]
       - ["System.Text", "StringBuilder", False, "AppendFormat", "(System.IFormatProvider,System.String,System.Object)", "", "Argument[this]", "ReturnValue", "value", "manual"]
@@ -97,6 +103,10 @@ extensions:
       - ["System.Text", "StringBuilder", False, "AppendLine", "()", "", "Argument[this]", "ReturnValue", "value", "manual"]
       - ["System.Text", "StringBuilder", False, "AppendLine", "(System.String)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
       - ["System.Text", "StringBuilder", False, "AppendLine", "(System.String)", "", "Argument[this]", "ReturnValue", "value", "manual"]
+      - ["System.Text", "StringBuilder", False, "AppendLine", "(System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[this]", "ReturnValue", "value", "manual"]
+      - ["System.Text", "StringBuilder", False, "AppendLine", "(System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
+      - ["System.Text", "StringBuilder", False, "AppendLine", "(System.IFormatProvider,System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[this]", "ReturnValue", "value", "manual"]
+      - ["System.Text", "StringBuilder", False, "AppendLine", "(System.IFormatProvider,System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[1]", "Argument[this]", "taint", "manual"]
       - ["System.Text", "StringBuilder", False, "StringBuilder", "(System.String)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
       - ["System.Text", "StringBuilder", False, "StringBuilder", "(System.String,System.Int32)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
       - ["System.Text", "StringBuilder", False, "StringBuilder", "(System.String,System.Int32,System.Int32,System.Int32)", "", "Argument[0]", "Argument[this]", "taint", "manual"]

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

@@ -87,6 +87,19 @@ private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityCon
       or
       e1 = e2.(AwaitExpr).getExpr() and
       scope = e2
+      or
+      // Taint flows from the operand of a cast to the cast expression if the cast is to an interpolated string handler.
+      e2 =
+        any(CastExpr ce |
+          e1 = ce.getExpr() and
+          scope = ce and
+          ce.getTargetType()
+              .(Attributable)
+              .getAnAttribute()
+              .getType()
+              .hasFullyQualifiedName("System.Runtime.CompilerServices",
+                "InterpolatedStringHandlerAttribute")
+        )
     )
   }
 }

csharp/ql/test/library-tests/dataflow/global/DataFlow.expected

@@ -51,21 +51,21 @@
 | GlobalDataFlow.cs:329:15:329:25 | access to parameter sinkParam11 |
 | GlobalDataFlow.cs:404:15:404:20 | access to local variable sink11 |
 | GlobalDataFlow.cs:427:41:427:46 | access to local variable sink20 |
-| GlobalDataFlow.cs:478:15:478:20 | access to local variable sink45 |
-| GlobalDataFlow.cs:486:32:486:32 | access to parameter s |
-| GlobalDataFlow.cs:508:15:508:22 | access to field field |
-| GlobalDataFlow.cs:509:15:509:22 | access to field field |
-| GlobalDataFlow.cs:515:15:515:22 | access to field field |
-| GlobalDataFlow.cs:516:15:516:22 | access to field field |
-| GlobalDataFlow.cs:517:15:517:22 | access to field field |
-| GlobalDataFlow.cs:526:15:526:21 | access to field field |
+| GlobalDataFlow.cs:461:15:461:20 | access to local variable sink45 |
+| GlobalDataFlow.cs:469:32:469:32 | access to parameter s |
+| GlobalDataFlow.cs:491:15:491:22 | access to field field |
+| GlobalDataFlow.cs:492:15:492:22 | access to field field |
+| GlobalDataFlow.cs:498:15:498:22 | access to field field |
+| GlobalDataFlow.cs:499:15:499:22 | access to field field |
+| GlobalDataFlow.cs:500:15:500:22 | access to field field |
+| GlobalDataFlow.cs:509:15:509:21 | access to field field |
+| GlobalDataFlow.cs:516:15:516:21 | access to field field |
+| GlobalDataFlow.cs:517:15:517:21 | access to field field |
+| GlobalDataFlow.cs:531:15:531:21 | access to field field |
+| GlobalDataFlow.cs:532:15:532:21 | access to field field |
 | GlobalDataFlow.cs:533:15:533:21 | access to field field |
-| GlobalDataFlow.cs:534:15:534:21 | access to field field |
-| GlobalDataFlow.cs:548:15:548:21 | access to field field |
-| GlobalDataFlow.cs:549:15:549:21 | access to field field |
-| GlobalDataFlow.cs:550:15:550:21 | access to field field |
-| GlobalDataFlow.cs:556:15:556:22 | access to field field |
-| GlobalDataFlow.cs:564:15:564:21 | access to field field |
+| GlobalDataFlow.cs:539:15:539:22 | access to field field |
+| GlobalDataFlow.cs:547:15:547:21 | access to field field |
 | Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x |
 | Splitting.cs:9:15:9:15 | [b (line 3): true] access to local variable x |
 | Splitting.cs:11:19:11:19 | access to local variable x |