Skip to content

Commit 51adcf5

Browse files
tamasvajktamasvajk
authored
Merge pull request #15010 from tamasvajk/fix/stringbuilder-interpolation
C#: Support interpolated strings in `StringBuilder.Append`
2 parents 32fdf4f + 75fa677 commit 51adcf5

File tree

12 files changed

+494
-369
lines changed

12 files changed

+494
-369
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
5+
* 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‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,16 @@ extensions:
4545
- ["System.Text", "StringBuilder", False, "Append", "(System.String,System.Int32,System.Int32)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
4646
- ["System.Text", "StringBuilder", False, "Append", "(System.String,System.Int32,System.Int32)", "", "Argument[this]", "ReturnValue", "value", "manual"]
4747
- ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder)", "", "Argument[this]", "ReturnValue", "value", "manual"]
48+
- ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
4849
- ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder,System.Int32,System.Int32)", "", "Argument[this]", "ReturnValue", "value", "manual"]
50+
- ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder,System.Int32,System.Int32)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
4951
- ["System.Text", "StringBuilder", False, "Append", "(System.UInt16)", "", "Argument[this]", "ReturnValue", "value", "manual"]
5052
- ["System.Text", "StringBuilder", False, "Append", "(System.UInt32)", "", "Argument[this]", "ReturnValue", "value", "manual"]
5153
- ["System.Text", "StringBuilder", False, "Append", "(System.UInt64)", "", "Argument[this]", "ReturnValue", "value", "manual"]
54+
- ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[this]", "ReturnValue", "value", "manual"]
55+
- ["System.Text", "StringBuilder", False, "Append", "(System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
56+
- ["System.Text", "StringBuilder", False, "Append", "(System.IFormatProvider,System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[this]", "ReturnValue", "value", "manual"]
57+
- ["System.Text", "StringBuilder", False, "Append", "(System.IFormatProvider,System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[1]", "Argument[this]", "taint", "manual"]
5258
- ["System.Text", "StringBuilder", False, "AppendFormat", "(System.IFormatProvider,System.String,System.Object)", "", "Argument[1]", "Argument[this]", "taint", "manual"]
5359
- ["System.Text", "StringBuilder", False, "AppendFormat", "(System.IFormatProvider,System.String,System.Object)", "", "Argument[2]", "Argument[this]", "taint", "manual"]
5460
- ["System.Text", "StringBuilder", False, "AppendFormat", "(System.IFormatProvider,System.String,System.Object)", "", "Argument[this]", "ReturnValue", "value", "manual"]
@@ -97,6 +103,10 @@ extensions:
97103
- ["System.Text", "StringBuilder", False, "AppendLine", "()", "", "Argument[this]", "ReturnValue", "value", "manual"]
98104
- ["System.Text", "StringBuilder", False, "AppendLine", "(System.String)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
99105
- ["System.Text", "StringBuilder", False, "AppendLine", "(System.String)", "", "Argument[this]", "ReturnValue", "value", "manual"]
106+
- ["System.Text", "StringBuilder", False, "AppendLine", "(System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[this]", "ReturnValue", "value", "manual"]
107+
- ["System.Text", "StringBuilder", False, "AppendLine", "(System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
108+
- ["System.Text", "StringBuilder", False, "AppendLine", "(System.IFormatProvider,System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[this]", "ReturnValue", "value", "manual"]
109+
- ["System.Text", "StringBuilder", False, "AppendLine", "(System.IFormatProvider,System.Text.StringBuilder+AppendInterpolatedStringHandler)", "", "Argument[1]", "Argument[this]", "taint", "manual"]
100110
- ["System.Text", "StringBuilder", False, "StringBuilder", "(System.String)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
101111
- ["System.Text", "StringBuilder", False, "StringBuilder", "(System.String,System.Int32)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
102112
- ["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‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,19 @@ private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityCon
8787
or
8888
e1 = e2.(AwaitExpr).getExpr() and
8989
scope = e2
90+
or
91+
// Taint flows from the operand of a cast to the cast expression if the cast is to an interpolated string handler.
92+
e2 =
93+
any(CastExpr ce |
94+
e1 = ce.getExpr() and
95+
scope = ce and
96+
ce.getTargetType()
97+
.(Attributable)
98+
.getAnAttribute()
99+
.getType()
100+
.hasFullyQualifiedName("System.Runtime.CompilerServices",
101+
"InterpolatedStringHandlerAttribute")
102+
)
90103
)
91104
}
92105
}

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,21 +51,21 @@
5151
| GlobalDataFlow.cs:329:15:329:25 | access to parameter sinkParam11 |
5252
| GlobalDataFlow.cs:404:15:404:20 | access to local variable sink11 |
5353
| GlobalDataFlow.cs:427:41:427:46 | access to local variable sink20 |
54-
| GlobalDataFlow.cs:478:15:478:20 | access to local variable sink45 |
55-
| GlobalDataFlow.cs:486:32:486:32 | access to parameter s |
56-
| GlobalDataFlow.cs:508:15:508:22 | access to field field |
57-
| GlobalDataFlow.cs:509:15:509:22 | access to field field |
58-
| GlobalDataFlow.cs:515:15:515:22 | access to field field |
59-
| GlobalDataFlow.cs:516:15:516:22 | access to field field |
60-
| GlobalDataFlow.cs:517:15:517:22 | access to field field |
61-
| GlobalDataFlow.cs:526:15:526:21 | access to field field |
54+
| GlobalDataFlow.cs:461:15:461:20 | access to local variable sink45 |
55+
| GlobalDataFlow.cs:469:32:469:32 | access to parameter s |
56+
| GlobalDataFlow.cs:491:15:491:22 | access to field field |
57+
| GlobalDataFlow.cs:492:15:492:22 | access to field field |
58+
| GlobalDataFlow.cs:498:15:498:22 | access to field field |
59+
| GlobalDataFlow.cs:499:15:499:22 | access to field field |
60+
| GlobalDataFlow.cs:500:15:500:22 | access to field field |
61+
| GlobalDataFlow.cs:509:15:509:21 | access to field field |
62+
| GlobalDataFlow.cs:516:15:516:21 | access to field field |
63+
| GlobalDataFlow.cs:517:15:517:21 | access to field field |
64+
| GlobalDataFlow.cs:531:15:531:21 | access to field field |
65+
| GlobalDataFlow.cs:532:15:532:21 | access to field field |
6266
| GlobalDataFlow.cs:533:15:533:21 | access to field field |
63-
| GlobalDataFlow.cs:534:15:534:21 | access to field field |
64-
| GlobalDataFlow.cs:548:15:548:21 | access to field field |
65-
| GlobalDataFlow.cs:549:15:549:21 | access to field field |
66-
| GlobalDataFlow.cs:550:15:550:21 | access to field field |
67-
| GlobalDataFlow.cs:556:15:556:22 | access to field field |
68-
| GlobalDataFlow.cs:564:15:564:21 | access to field field |
67+
| GlobalDataFlow.cs:539:15:539:22 | access to field field |
68+
| GlobalDataFlow.cs:547:15:547:21 | access to field field |
6969
| Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x |
7070
| Splitting.cs:9:15:9:15 | [b (line 3): true] access to local variable x |
7171
| Splitting.cs:11:19:11:19 | access to local variable x |

0 commit comments

Comments
 (0)