Skip to content

Commit 89df59a

Browse files
tamasvajktamasvajk
committed
C#: Add missing models and fix interpolated string flow into StringBuilder 
1 parent 2c624c2 commit 89df59a

File tree

6 files changed

+85
-8
lines changed

6 files changed

+85
-8
lines changed

‎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[0]", "ReturnValue", "taint", "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[1]", "ReturnValue", "taint", "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[0]", "ReturnValue", "taint", "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[1]", "ReturnValue", "taint", "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/TaintTracking.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@
7373
| GlobalDataFlow.cs:539:15:539:22 | access to field field |
7474
| GlobalDataFlow.cs:547:15:547:21 | access to field field |
7575
| GlobalDataFlowStringBuilder.cs:32:15:32:19 | access to local variable sink0 |
76+
| GlobalDataFlowStringBuilder.cs:37:15:37:19 | access to local variable sink1 |
77+
| GlobalDataFlowStringBuilder.cs:42:15:42:19 | access to local variable sink2 |
78+
| GlobalDataFlowStringBuilder.cs:50:15:50:19 | access to local variable sink3 |
7679
| Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x |
7780
| Splitting.cs:9:15:9:15 | [b (line 3): true] access to local variable x |
7881
| Splitting.cs:11:19:11:19 | access to local variable x |

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,11 +328,28 @@ edges
328328
| GlobalDataFlow.cs:558:46:558:46 | access to local variable x : String | GlobalDataFlow.cs:558:44:558:47 | delegate call : String |
329329
| GlobalDataFlowStringBuilder.cs:17:64:17:64 | s : String | GlobalDataFlowStringBuilder.cs:19:19:19:19 | access to parameter s : String |
330330
| GlobalDataFlowStringBuilder.cs:19:19:19:19 | access to parameter s : String | GlobalDataFlowStringBuilder.cs:19:9:19:10 | [post] access to parameter sb : StringBuilder |
331+
| GlobalDataFlowStringBuilder.cs:22:76:22:76 | s : String | GlobalDataFlowStringBuilder.cs:24:19:24:26 | (...) ... : AppendInterpolatedStringHandler |
332+
| GlobalDataFlowStringBuilder.cs:24:19:24:26 | (...) ... : AppendInterpolatedStringHandler | GlobalDataFlowStringBuilder.cs:24:9:24:10 | [post] access to parameter sb : StringBuilder |
331333
| GlobalDataFlowStringBuilder.cs:30:31:30:32 | [post] access to local variable sb : StringBuilder | GlobalDataFlowStringBuilder.cs:31:21:31:22 | access to local variable sb : StringBuilder |
334+
| GlobalDataFlowStringBuilder.cs:30:31:30:32 | [post] access to local variable sb : StringBuilder | GlobalDataFlowStringBuilder.cs:35:20:35:21 | access to local variable sb : StringBuilder |
335+
| GlobalDataFlowStringBuilder.cs:30:31:30:32 | [post] access to local variable sb : StringBuilder | GlobalDataFlowStringBuilder.cs:40:20:40:26 | (...) ... : AppendInterpolatedStringHandler |
332336
| GlobalDataFlowStringBuilder.cs:30:35:30:48 | "taint source" : String | GlobalDataFlowStringBuilder.cs:17:64:17:64 | s : String |
333337
| GlobalDataFlowStringBuilder.cs:30:35:30:48 | "taint source" : String | GlobalDataFlowStringBuilder.cs:30:31:30:32 | [post] access to local variable sb : StringBuilder |
334338
| GlobalDataFlowStringBuilder.cs:31:21:31:22 | access to local variable sb : StringBuilder | GlobalDataFlowStringBuilder.cs:31:21:31:33 | call to method ToString : String |
335339
| GlobalDataFlowStringBuilder.cs:31:21:31:33 | call to method ToString : String | GlobalDataFlowStringBuilder.cs:32:15:32:19 | access to local variable sink0 |
340+
| GlobalDataFlowStringBuilder.cs:35:9:35:11 | [post] access to local variable sb1 : StringBuilder | GlobalDataFlowStringBuilder.cs:36:21:36:23 | access to local variable sb1 : StringBuilder |
341+
| GlobalDataFlowStringBuilder.cs:35:20:35:21 | access to local variable sb : StringBuilder | GlobalDataFlowStringBuilder.cs:35:9:35:11 | [post] access to local variable sb1 : StringBuilder |
342+
| GlobalDataFlowStringBuilder.cs:36:21:36:23 | access to local variable sb1 : StringBuilder | GlobalDataFlowStringBuilder.cs:36:21:36:34 | call to method ToString : String |
343+
| GlobalDataFlowStringBuilder.cs:36:21:36:34 | call to method ToString : String | GlobalDataFlowStringBuilder.cs:37:15:37:19 | access to local variable sink1 |
344+
| GlobalDataFlowStringBuilder.cs:40:9:40:11 | [post] access to local variable sb2 : StringBuilder | GlobalDataFlowStringBuilder.cs:41:21:41:23 | access to local variable sb2 : StringBuilder |
345+
| GlobalDataFlowStringBuilder.cs:40:20:40:26 | (...) ... : AppendInterpolatedStringHandler | GlobalDataFlowStringBuilder.cs:40:9:40:11 | [post] access to local variable sb2 : StringBuilder |
346+
| GlobalDataFlowStringBuilder.cs:41:21:41:23 | access to local variable sb2 : StringBuilder | GlobalDataFlowStringBuilder.cs:41:21:41:34 | call to method ToString : String |
347+
| GlobalDataFlowStringBuilder.cs:41:21:41:34 | call to method ToString : String | GlobalDataFlowStringBuilder.cs:42:15:42:19 | access to local variable sink2 |
348+
| GlobalDataFlowStringBuilder.cs:48:43:48:44 | [post] access to local variable sb : StringBuilder | GlobalDataFlowStringBuilder.cs:49:21:49:22 | access to local variable sb : StringBuilder |
349+
| GlobalDataFlowStringBuilder.cs:48:47:48:60 | "taint source" : String | GlobalDataFlowStringBuilder.cs:22:76:22:76 | s : String |
350+
| GlobalDataFlowStringBuilder.cs:48:47:48:60 | "taint source" : String | GlobalDataFlowStringBuilder.cs:48:43:48:44 | [post] access to local variable sb : StringBuilder |
351+
| GlobalDataFlowStringBuilder.cs:49:21:49:22 | access to local variable sb : StringBuilder | GlobalDataFlowStringBuilder.cs:49:21:49:33 | call to method ToString : String |
352+
| GlobalDataFlowStringBuilder.cs:49:21:49:33 | call to method ToString : String | GlobalDataFlowStringBuilder.cs:50:15:50:19 | access to local variable sink3 |
336353
| Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:8:24:8:30 | [b (line 3): false] access to parameter tainted : String |
337354
| Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:8:24:8:30 | [b (line 3): true] access to parameter tainted : String |
338355
| Splitting.cs:8:17:8:31 | [b (line 3): false] call to method Return<String> : String | Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x |
@@ -657,11 +674,29 @@ nodes
657674
| GlobalDataFlowStringBuilder.cs:17:64:17:64 | s : String | semmle.label | s : String |
658675
| GlobalDataFlowStringBuilder.cs:19:9:19:10 | [post] access to parameter sb : StringBuilder | semmle.label | [post] access to parameter sb : StringBuilder |
659676
| GlobalDataFlowStringBuilder.cs:19:19:19:19 | access to parameter s : String | semmle.label | access to parameter s : String |
677+
| GlobalDataFlowStringBuilder.cs:22:76:22:76 | s : String | semmle.label | s : String |
678+
| GlobalDataFlowStringBuilder.cs:24:9:24:10 | [post] access to parameter sb : StringBuilder | semmle.label | [post] access to parameter sb : StringBuilder |
679+
| GlobalDataFlowStringBuilder.cs:24:19:24:26 | (...) ... : AppendInterpolatedStringHandler | semmle.label | (...) ... : AppendInterpolatedStringHandler |
660680
| GlobalDataFlowStringBuilder.cs:30:31:30:32 | [post] access to local variable sb : StringBuilder | semmle.label | [post] access to local variable sb : StringBuilder |
661681
| GlobalDataFlowStringBuilder.cs:30:35:30:48 | "taint source" : String | semmle.label | "taint source" : String |
662682
| GlobalDataFlowStringBuilder.cs:31:21:31:22 | access to local variable sb : StringBuilder | semmle.label | access to local variable sb : StringBuilder |
663683
| GlobalDataFlowStringBuilder.cs:31:21:31:33 | call to method ToString : String | semmle.label | call to method ToString : String |
664684
| GlobalDataFlowStringBuilder.cs:32:15:32:19 | access to local variable sink0 | semmle.label | access to local variable sink0 |
685+
| GlobalDataFlowStringBuilder.cs:35:9:35:11 | [post] access to local variable sb1 : StringBuilder | semmle.label | [post] access to local variable sb1 : StringBuilder |
686+
| GlobalDataFlowStringBuilder.cs:35:20:35:21 | access to local variable sb : StringBuilder | semmle.label | access to local variable sb : StringBuilder |
687+
| GlobalDataFlowStringBuilder.cs:36:21:36:23 | access to local variable sb1 : StringBuilder | semmle.label | access to local variable sb1 : StringBuilder |
688+
| GlobalDataFlowStringBuilder.cs:36:21:36:34 | call to method ToString : String | semmle.label | call to method ToString : String |
689+
| GlobalDataFlowStringBuilder.cs:37:15:37:19 | access to local variable sink1 | semmle.label | access to local variable sink1 |
690+
| GlobalDataFlowStringBuilder.cs:40:9:40:11 | [post] access to local variable sb2 : StringBuilder | semmle.label | [post] access to local variable sb2 : StringBuilder |
691+
| GlobalDataFlowStringBuilder.cs:40:20:40:26 | (...) ... : AppendInterpolatedStringHandler | semmle.label | (...) ... : AppendInterpolatedStringHandler |
692+
| GlobalDataFlowStringBuilder.cs:41:21:41:23 | access to local variable sb2 : StringBuilder | semmle.label | access to local variable sb2 : StringBuilder |
693+
| GlobalDataFlowStringBuilder.cs:41:21:41:34 | call to method ToString : String | semmle.label | call to method ToString : String |
694+
| GlobalDataFlowStringBuilder.cs:42:15:42:19 | access to local variable sink2 | semmle.label | access to local variable sink2 |
695+
| GlobalDataFlowStringBuilder.cs:48:43:48:44 | [post] access to local variable sb : StringBuilder | semmle.label | [post] access to local variable sb : StringBuilder |
696+
| GlobalDataFlowStringBuilder.cs:48:47:48:60 | "taint source" : String | semmle.label | "taint source" : String |
697+
| GlobalDataFlowStringBuilder.cs:49:21:49:22 | access to local variable sb : StringBuilder | semmle.label | access to local variable sb : StringBuilder |
698+
| GlobalDataFlowStringBuilder.cs:49:21:49:33 | call to method ToString : String | semmle.label | call to method ToString : String |
699+
| GlobalDataFlowStringBuilder.cs:50:15:50:19 | access to local variable sink3 | semmle.label | access to local variable sink3 |
665700
| Splitting.cs:3:28:3:34 | tainted : String | semmle.label | tainted : String |
666701
| Splitting.cs:8:17:8:31 | [b (line 3): false] call to method Return<String> : String | semmle.label | [b (line 3): false] call to method Return<String> : String |
667702
| Splitting.cs:8:17:8:31 | [b (line 3): true] call to method Return<String> : String | semmle.label | [b (line 3): true] call to method Return<String> : String |
@@ -709,6 +744,7 @@ subpaths
709744
| GlobalDataFlow.cs:389:18:389:18 | access to parameter x : String | GlobalDataFlow.cs:300:27:300:28 | x0 : String | GlobalDataFlow.cs:300:33:300:34 | access to parameter x0 : String | GlobalDataFlow.cs:389:16:389:19 | delegate call : String |
710745
| GlobalDataFlow.cs:558:46:558:46 | access to local variable x : String | GlobalDataFlow.cs:81:79:81:79 | x : String | GlobalDataFlow.cs:81:84:81:84 | access to parameter x : String | GlobalDataFlow.cs:558:44:558:47 | delegate call : String |
711746
| GlobalDataFlowStringBuilder.cs:30:35:30:48 | "taint source" : String | GlobalDataFlowStringBuilder.cs:17:64:17:64 | s : String | GlobalDataFlowStringBuilder.cs:19:9:19:10 | [post] access to parameter sb : StringBuilder | GlobalDataFlowStringBuilder.cs:30:31:30:32 | [post] access to local variable sb : StringBuilder |
747+
| GlobalDataFlowStringBuilder.cs:48:47:48:60 | "taint source" : String | GlobalDataFlowStringBuilder.cs:22:76:22:76 | s : String | GlobalDataFlowStringBuilder.cs:24:9:24:10 | [post] access to parameter sb : StringBuilder | GlobalDataFlowStringBuilder.cs:48:43:48:44 | [post] access to local variable sb : StringBuilder |
712748
| Splitting.cs:8:24:8:30 | [b (line 3): false] access to parameter tainted : String | Splitting.cs:16:26:16:26 | x : String | Splitting.cs:16:32:16:32 | access to parameter x : String | Splitting.cs:8:17:8:31 | [b (line 3): false] call to method Return<String> : String |
713749
| Splitting.cs:8:24:8:30 | [b (line 3): true] access to parameter tainted : String | Splitting.cs:16:26:16:26 | x : String | Splitting.cs:16:32:16:32 | access to parameter x : String | Splitting.cs:8:17:8:31 | [b (line 3): true] call to method Return<String> : String |
714750
| Splitting.cs:20:29:20:29 | access to parameter s : String | Splitting.cs:16:26:16:26 | x : String | Splitting.cs:16:32:16:32 | access to parameter x : String | Splitting.cs:20:22:20:30 | call to method Return<String> : String |
@@ -791,6 +827,9 @@ subpaths
791827
| GlobalDataFlow.cs:539:15:539:22 | access to field field | GlobalDataFlow.cs:483:20:483:33 | "taint source" : String | GlobalDataFlow.cs:539:15:539:22 | access to field field | access to field field |
792828
| GlobalDataFlow.cs:547:15:547:21 | access to field field | GlobalDataFlow.cs:483:20:483:33 | "taint source" : String | GlobalDataFlow.cs:547:15:547:21 | access to field field | access to field field |
793829
| GlobalDataFlowStringBuilder.cs:32:15:32:19 | access to local variable sink0 | GlobalDataFlowStringBuilder.cs:30:35:30:48 | "taint source" : String | GlobalDataFlowStringBuilder.cs:32:15:32:19 | access to local variable sink0 | access to local variable sink0 |
830+
| GlobalDataFlowStringBuilder.cs:37:15:37:19 | access to local variable sink1 | GlobalDataFlowStringBuilder.cs:30:35:30:48 | "taint source" : String | GlobalDataFlowStringBuilder.cs:37:15:37:19 | access to local variable sink1 | access to local variable sink1 |
831+
| GlobalDataFlowStringBuilder.cs:42:15:42:19 | access to local variable sink2 | GlobalDataFlowStringBuilder.cs:30:35:30:48 | "taint source" : String | GlobalDataFlowStringBuilder.cs:42:15:42:19 | access to local variable sink2 | access to local variable sink2 |
832+
| GlobalDataFlowStringBuilder.cs:50:15:50:19 | access to local variable sink3 | GlobalDataFlowStringBuilder.cs:48:47:48:60 | "taint source" : String | GlobalDataFlowStringBuilder.cs:50:15:50:19 | access to local variable sink3 | access to local variable sink3 |
794833
| Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x | [b (line 3): false] access to local variable x |
795834
| Splitting.cs:9:15:9:15 | [b (line 3): true] access to local variable x | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:9:15:9:15 | [b (line 3): true] access to local variable x | [b (line 3): true] access to local variable x |
796835
| Splitting.cs:11:19:11:19 | access to local variable x | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:11:19:11:19 | access to local variable x | access to local variable x |

0 commit comments

Comments
 (0)