Skip to content

Commit 5de6283

Browse files
michaelnebelmichaelnebel
authored
Merge pull request #22628 from forks-felickz/felickz-csharp-razor-tag-helper-xss-fp
C#: Fix cs/web/xss false positive on Razor tag-helper attribute values
2 parents 6230d2f + 79fae82 commit 5de6283

6 files changed

Lines changed: 195 additions & 2 deletions

File tree

‎csharp/ql/lib/change-notes/2026-09-18-razor-tag-helper-attribute-xss-fp.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: majorAnalysis
3+
---
4+
* Fixed a false positive in `cs/web/xss` for ASP.NET Core Razor Pages/MVC views: `WriteLiteral` calls generated for tag helper attribute values (for example, `asp-for`) capture the value into an internal buffer instead of writing it directly to the response, so they are no longer treated as XSS sinks.

‎csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,16 @@ class MicrosoftAspNetCoreMvcRazorPageBase extends Class {
491491

492492
/** Gets the `WriteLiteral` method. */
493493
Method getWriteLiteralMethod() { result = this.getAMethod("WriteLiteral") }
494+
495+
/** Gets the `BeginWriteTagHelperAttribute` method. */
496+
Method getBeginWriteTagHelperAttributeMethod() {
497+
result = this.getAMethod("BeginWriteTagHelperAttribute")
498+
}
499+
500+
/** Gets the `EndWriteTagHelperAttribute` method. */
501+
Method getEndWriteTagHelperAttributeMethod() {
502+
result = this.getAMethod("EndWriteTagHelperAttribute")
503+
}
494504
}
495505

496506
/** A class deriving from `Microsoft.AspNetCore.Http.HttpRequest`, implements `HttpRequest` in ASP.NET Core. */

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/Html.qll‎

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,14 +177,94 @@ class MicrosoftAspNetCoreMvcHtmlHelperRawSink extends AspNetCoreHtmlSink {
177177
}
178178
}
179179

180+
/**
181+
* Holds if `writeLiteral` is a call to `RazorPageBase.WriteLiteral` whose argument is captured
182+
* between a matching pair of `BeginWriteTagHelperAttribute()`/`EndWriteTagHelperAttribute()`
183+
* calls, in the same basic block, with no other such calls in between.
184+
*
185+
* The Razor source generator emits this bracketing for every literal or expression segment of an
186+
* HTML attribute value on an element that also carries a tag helper (for example `asp-for`). Such
187+
* a `WriteLiteral` call does not write directly to the response: `WriteLiteral` appends to an
188+
* internal string buffer, and `EndWriteTagHelperAttribute()` returns that buffer as a tag helper
189+
* attribute value rather than as page markup. This is therefore not a direct-write sink, unlike an
190+
* unbracketed `WriteLiteral` call, whose argument is written straight to the response.
191+
*
192+
* Because a basic block cannot contain a branch, requiring `beginCall`, `writeLiteral`, and
193+
* `endCall` to appear (in that order) in the same basic block, with no other
194+
* `Begin`/`EndWriteTagHelperAttribute` call strictly between `beginCall` and `endCall`,
195+
* guarantees that `beginCall`/`endCall` are the immediately enclosing bracket around
196+
* `writeLiteral` on every path that reaches it (that is, the bracket opened by `beginCall` is
197+
* still open, and not yet closed by some other `endCall`, at the point `writeLiteral` executes).
198+
* No other such call can coincide with `writeLiteral` itself, so checking the whole open interval
199+
* between `beginCall` and `endCall` is equivalent to checking it on both sides of `writeLiteral`
200+
* separately.
201+
*
202+
* `beginCall`, `writeLiteral`, and `endCall` are additionally required to have an implicit `this`
203+
* qualifier, which is how the Razor source generator always emits these calls. Combined with all
204+
* three calls being required to lie in the same basic block (and hence the same method body),
205+
* this ensures all three calls act on the same page instance, so a bracket on one page cannot be
206+
* mistaken for a bracket around a `WriteLiteral` call on a different page.
207+
*
208+
* The `WriteLiteral`/`BeginWriteTagHelperAttribute`/`EndWriteTagHelperAttribute` methods are
209+
* looked up via `any(MicrosoftAspNetCoreMvcRazorPageBase page).get...Method()` rather than
210+
* through a single shared `page` variable bound across `writeLiteral`, `beginCall`, and
211+
* `endCall`. These methods are inherited (not overridden) from the shared `RazorPageBase`
212+
* framework type, so every generated Razor page class resolves to the same handful of `Method`
213+
* entities; binding a single `page` variable across all three calls would force the join to be
214+
* repeated once per generated page class in the codebase, rather than once per distinct `Method`,
215+
* causing severe performance degradation on codebases with many Razor views.
216+
*/
217+
private predicate isBracketedForTagHelperAttribute(Call writeLiteral) {
218+
exists(MethodCall beginCall, MethodCall endCall, BasicBlock bb, int i, int j, int k |
219+
bb = writeLiteral.getBasicBlock() and
220+
writeLiteral = any(MicrosoftAspNetCoreMvcRazorPageBase page).getWriteLiteralMethod().getACall() and
221+
beginCall =
222+
any(MicrosoftAspNetCoreMvcRazorPageBase page)
223+
.getBeginWriteTagHelperAttributeMethod()
224+
.getACall() and
225+
endCall =
226+
any(MicrosoftAspNetCoreMvcRazorPageBase page).getEndWriteTagHelperAttributeMethod().getACall() and
227+
writeLiteral.(QualifiableExpr).hasImplicitThisQualifier() and
228+
beginCall.hasImplicitThisQualifier() and
229+
endCall.hasImplicitThisQualifier() and
230+
bb.getNode(i) = beginCall.getControlFlowNode() and
231+
bb.getNode(j) = writeLiteral.getControlFlowNode() and
232+
bb.getNode(k) = endCall.getControlFlowNode() and
233+
i < j and
234+
j < k and
235+
not exists(int l, Call other |
236+
(
237+
other =
238+
any(MicrosoftAspNetCoreMvcRazorPageBase page)
239+
.getBeginWriteTagHelperAttributeMethod()
240+
.getACall() or
241+
other =
242+
any(MicrosoftAspNetCoreMvcRazorPageBase page)
243+
.getEndWriteTagHelperAttributeMethod()
244+
.getACall()
245+
) and
246+
bb.getNode(l) = other.getControlFlowNode() and
247+
i < l and
248+
l < k
249+
)
250+
)
251+
}
252+
180253
/**
181254
* An expression that is used as an argument to `Page.WriteLiteral` in ASP.NET 6.0 razor page, typically in
182255
* a `.cshtml` file.
256+
*
257+
* `WriteLiteral` calls whose argument is captured for a tag helper attribute value (see
258+
* `isBracketedForTagHelperAttribute`) are excluded, since such calls buffer the value as a tag
259+
* helper attribute rather than writing it directly to the response.
183260
*/
184261
class MicrosoftAspNetRazorPageWriteLiteralSink extends AspNetCoreHtmlSink {
185262
MicrosoftAspNetRazorPageWriteLiteralSink() {
186-
this.getExpr() =
187-
any(MicrosoftAspNetCoreMvcRazorPageBase h).getWriteLiteralMethod().getACall().getAnArgument()
263+
exists(Call writeLiteral |
264+
writeLiteral = any(MicrosoftAspNetCoreMvcRazorPageBase h).getWriteLiteralMethod().getACall() and
265+
this.getExpr() = writeLiteral.getAnArgument() and
266+
not isBracketedForTagHelperAttribute(writeLiteral)
267+
)
188268
}
189269
}
190270

‎csharp/ql/test/query-tests/Security Features/CWE-079/XSS/RazorTagHelperAttribute.cshtml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
// empty

‎csharp/ql/test/query-tests/Security Features/CWE-079/XSS/RazorTagHelperAttribute.cshtml.g.cs‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// A hand-written test file that mimics the output of compiling a `.cshtml` file that has an
2+
// element with a tag helper (e.g. `asp-for`) and an HTML attribute whose value contains a
3+
// tainted expression, for example: <input asp-for="Value" value="@Model.Value">
4+
#pragma checksum "RazorTagHelperAttribute.cshtml" "{ff1816ec-aa5e-4d10-87f7-6f4963833460}" "c4ae76542f1958092cebd8f57beef899d20fc548"
5+
// <auto-generated/>
6+
#pragma warning disable 1591
7+
[assembly: global::Microsoft.AspNetCore.Razor.Hosting.RazorCompiledItemAttribute(typeof(dotnetweb.Pages.Pages_RazorTagHelperAttribute), @"mvc.1.0.razor-page", @"RazorTagHelperAttribute.cshtml")]
8+
namespace dotnetweb.Pages
9+
{
10+
#line hidden
11+
using System;
12+
using System.Collections.Generic;
13+
using System.Linq;
14+
using System.Threading.Tasks;
15+
using Microsoft.AspNetCore.Mvc;
16+
using Microsoft.AspNetCore.Mvc.Rendering;
17+
using Microsoft.AspNetCore.Mvc.ViewFeatures;
18+
#nullable restore
19+
using dotnetweb;
20+
21+
#line default
22+
#line hidden
23+
#nullable disable
24+
[global::Microsoft.AspNetCore.Razor.Hosting.RazorSourceChecksumAttribute(@"SHA1", @"c4ae76542f1958092cebd8f57beef899d20fc548", @"RazorTagHelperAttribute.cshtml")]
25+
public class Pages_RazorTagHelperAttribute : global::Microsoft.AspNetCore.Mvc.RazorPages.Page
26+
{
27+
#pragma warning disable 1998
28+
public async override global::System.Threading.Tasks.Task ExecuteAsync()
29+
{
30+
#nullable restore
31+
#line 3 "RazorTagHelperAttribute.cshtml"
32+
33+
var model = Request.Query["m"]; // $ Source=model
34+
35+
#line default
36+
#line hidden
37+
#nullable disable
38+
WriteLiteral("<input type=\"text\" ");
39+
// GOOD: this `WriteLiteral` call is generated for the value of an HTML attribute on
40+
// an element with a tag helper (e.g. `asp-for`). It writes into an internal string
41+
// buffer via `BeginWriteTagHelperAttribute()`/`EndWriteTagHelperAttribute()`, and the
42+
// buffered text is HTML-attribute-encoded later, when the tag helper attribute value
43+
// is rendered. It is not a direct, unencoded write, so it must not be flagged.
44+
BeginWriteTagHelperAttribute();
45+
WriteLiteral(model);
46+
var __tagHelperAttribute_1 = EndWriteTagHelperAttribute();
47+
WriteLiteral(" />");
48+
49+
// BAD: a `WriteLiteral` call that is not bracketed by
50+
// `BeginWriteTagHelperAttribute()`/`EndWriteTagHelperAttribute()` writes directly,
51+
// unencoded, to the response and must still be flagged.
52+
WriteLiteral(model); // $ Alert=model
53+
54+
// GOOD: two independent tag helper attribute brackets that occur one after another
55+
// in the same basic block must each be matched with their own nearest
56+
// `Begin`/`EndWriteTagHelperAttribute` call, and neither should leak into the other.
57+
BeginWriteTagHelperAttribute();
58+
WriteLiteral("literal-prefix-");
59+
var __tagHelperAttribute_2 = EndWriteTagHelperAttribute();
60+
BeginWriteTagHelperAttribute();
61+
WriteLiteral(model);
62+
var __tagHelperAttribute_3 = EndWriteTagHelperAttribute();
63+
64+
// BAD: a bare `WriteLiteral` call sandwiched between two unrelated tag helper
65+
// attribute brackets, in the same basic block, must not be mistaken for being
66+
// captured by either neighboring bracket.
67+
BeginWriteTagHelperAttribute();
68+
WriteLiteral("prefix");
69+
var __tagHelperAttribute_4 = EndWriteTagHelperAttribute();
70+
WriteLiteral(model); // $ Alert=model
71+
BeginWriteTagHelperAttribute();
72+
WriteLiteral("suffix");
73+
var __tagHelperAttribute_5 = EndWriteTagHelperAttribute();
74+
}
75+
#pragma warning restore 1998
76+
[global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
77+
public global::Microsoft.AspNetCore.Mvc.ViewFeatures.IModelExpressionProvider ModelExpressionProvider { get; private set; }
78+
[global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
79+
public global::Microsoft.AspNetCore.Mvc.IUrlHelper Url { get; private set; }
80+
[global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
81+
public global::Microsoft.AspNetCore.Mvc.IViewComponentHelper Component { get; private set; }
82+
[global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
83+
public global::Microsoft.AspNetCore.Mvc.Rendering.IJsonHelper Json { get; private set; }
84+
[global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
85+
public global::Microsoft.AspNetCore.Mvc.Rendering.IHtmlHelper<string> Html { get; private set; }
86+
public global::Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary<string> ViewData => (global::Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary<string>)PageContext?.ViewData;
87+
}
88+
}
89+
#pragma warning restore 1591

‎csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.expected‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#select
22
| Index.cshtml:14:16:14:22 | call to operator implicit conversion | Index.cshtml:5:19:5:31 | access to property Query : IQueryCollection | Index.cshtml:14:16:14:22 | call to operator implicit conversion | $@ flows to here and is written to HTML or JavaScript: Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method. | Index.cshtml:5:19:5:31 | access to property Query : IQueryCollection | User-provided value |
3+
| RazorTagHelperAttribute.cshtml.g.cs:52:26:52:30 | call to operator implicit conversion | RazorTagHelperAttribute.cshtml:4:17:4:29 | access to property Query : IQueryCollection | RazorTagHelperAttribute.cshtml.g.cs:52:26:52:30 | call to operator implicit conversion | $@ flows to here and is written to HTML or JavaScript: Microsoft.AspNetCore.Mvc.Razor.RazorPageBase.WriteLiteral() method. | RazorTagHelperAttribute.cshtml:4:17:4:29 | access to property Query : IQueryCollection | User-provided value |
4+
| RazorTagHelperAttribute.cshtml.g.cs:70:26:70:30 | call to operator implicit conversion | RazorTagHelperAttribute.cshtml:4:17:4:29 | access to property Query : IQueryCollection | RazorTagHelperAttribute.cshtml.g.cs:70:26:70:30 | call to operator implicit conversion | $@ flows to here and is written to HTML or JavaScript: Microsoft.AspNetCore.Mvc.Razor.RazorPageBase.WriteLiteral() method. | RazorTagHelperAttribute.cshtml:4:17:4:29 | access to property Query : IQueryCollection | User-provided value |
35
| XSSAspNet.cs:26:30:26:34 | access to local variable sayHi | XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | XSSAspNet.cs:26:30:26:34 | access to local variable sayHi | $@ flows to here and is written to HTML or JavaScript: System.Web.WebPages.WebPage.WriteLiteral() method. | XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | User-provided value |
46
| XSSAspNet.cs:36:40:36:44 | access to local variable sayHi | XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | XSSAspNet.cs:36:40:36:44 | access to local variable sayHi | $@ flows to here and is written to HTML or JavaScript: System.Web.WebPages.WebPage.WriteLiteralTo() method. | XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | User-provided value |
57
| XSSAspNet.cs:44:28:44:33 | access to local variable sayHi2 | XSSAspNet.cs:43:26:43:44 | access to property QueryString : NameValueCollection | XSSAspNet.cs:44:28:44:33 | access to local variable sayHi2 | $@ flows to here and is written to HTML or JavaScript. | XSSAspNet.cs:43:26:43:44 | access to property QueryString : NameValueCollection | User-provided value |
@@ -13,6 +15,9 @@
1315
edges
1416
| Index.cshtml:5:9:5:15 | access to local variable message : StringValues | Index.cshtml:14:16:14:22 | call to operator implicit conversion | provenance | |
1517
| Index.cshtml:5:19:5:31 | access to property Query : IQueryCollection | Index.cshtml:5:9:5:15 | access to local variable message : StringValues | provenance | |
18+
| RazorTagHelperAttribute.cshtml:4:9:4:13 | access to local variable model : StringValues | RazorTagHelperAttribute.cshtml.g.cs:52:26:52:30 | call to operator implicit conversion | provenance | |
19+
| RazorTagHelperAttribute.cshtml:4:9:4:13 | access to local variable model : StringValues | RazorTagHelperAttribute.cshtml.g.cs:70:26:70:30 | call to operator implicit conversion | provenance | |
20+
| RazorTagHelperAttribute.cshtml:4:17:4:29 | access to property Query : IQueryCollection | RazorTagHelperAttribute.cshtml:4:9:4:13 | access to local variable model : StringValues | provenance | |
1621
| XSSAspNet.cs:19:17:19:21 | access to local variable sayHi : String | XSSAspNet.cs:26:30:26:34 | access to local variable sayHi | provenance | |
1722
| XSSAspNet.cs:19:17:19:21 | access to local variable sayHi : String | XSSAspNet.cs:36:40:36:44 | access to local variable sayHi | provenance | |
1823
| XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | XSSAspNet.cs:19:17:19:21 | access to local variable sayHi : String | provenance | |
@@ -49,6 +54,10 @@ nodes
4954
| Index.cshtml:5:9:5:15 | access to local variable message : StringValues | semmle.label | access to local variable message : StringValues |
5055
| Index.cshtml:5:19:5:31 | access to property Query : IQueryCollection | semmle.label | access to property Query : IQueryCollection |
5156
| Index.cshtml:14:16:14:22 | call to operator implicit conversion | semmle.label | call to operator implicit conversion |
57+
| RazorTagHelperAttribute.cshtml.g.cs:52:26:52:30 | call to operator implicit conversion | semmle.label | call to operator implicit conversion |
58+
| RazorTagHelperAttribute.cshtml.g.cs:70:26:70:30 | call to operator implicit conversion | semmle.label | call to operator implicit conversion |
59+
| RazorTagHelperAttribute.cshtml:4:9:4:13 | access to local variable model : StringValues | semmle.label | access to local variable model : StringValues |
60+
| RazorTagHelperAttribute.cshtml:4:17:4:29 | access to property Query : IQueryCollection | semmle.label | access to property Query : IQueryCollection |
5261
| XSSAspNet.cs:19:17:19:21 | access to local variable sayHi : String | semmle.label | access to local variable sayHi : String |
5362
| XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
5463
| XSSAspNet.cs:19:25:19:52 | access to indexer : String | semmle.label | access to indexer : String |

0 commit comments

Comments
 (0)