diff --git a/csharp/ql/lib/change-notes/2026-09-18-razor-tag-helper-attribute-xss-fp.md b/csharp/ql/lib/change-notes/2026-09-18-razor-tag-helper-attribute-xss-fp.md new file mode 100644 index 000000000000..4cbe16158328 --- /dev/null +++ b/csharp/ql/lib/change-notes/2026-09-18-razor-tag-helper-attribute-xss-fp.md @@ -0,0 +1,4 @@ +--- +category: majorAnalysis +--- +* 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. diff --git a/csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll b/csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll index 37b0ff2884f9..f14eb2f96fa9 100644 --- a/csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll +++ b/csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll @@ -491,6 +491,16 @@ class MicrosoftAspNetCoreMvcRazorPageBase extends Class { /** Gets the `WriteLiteral` method. */ Method getWriteLiteralMethod() { result = this.getAMethod("WriteLiteral") } + + /** Gets the `BeginWriteTagHelperAttribute` method. */ + Method getBeginWriteTagHelperAttributeMethod() { + result = this.getAMethod("BeginWriteTagHelperAttribute") + } + + /** Gets the `EndWriteTagHelperAttribute` method. */ + Method getEndWriteTagHelperAttributeMethod() { + result = this.getAMethod("EndWriteTagHelperAttribute") + } } /** A class deriving from `Microsoft.AspNetCore.Http.HttpRequest`, implements `HttpRequest` in ASP.NET Core. */ diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/Html.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/Html.qll index 362a993e5321..bcc36a54abad 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/Html.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/Html.qll @@ -177,14 +177,94 @@ class MicrosoftAspNetCoreMvcHtmlHelperRawSink extends AspNetCoreHtmlSink { } } +/** + * Holds if `writeLiteral` is a call to `RazorPageBase.WriteLiteral` whose argument is captured + * between a matching pair of `BeginWriteTagHelperAttribute()`/`EndWriteTagHelperAttribute()` + * calls, in the same basic block, with no other such calls in between. + * + * The Razor source generator emits this bracketing for every literal or expression segment of an + * HTML attribute value on an element that also carries a tag helper (for example `asp-for`). Such + * a `WriteLiteral` call does not write directly to the response: `WriteLiteral` appends to an + * internal string buffer, and `EndWriteTagHelperAttribute()` returns that buffer as a tag helper + * attribute value rather than as page markup. This is therefore not a direct-write sink, unlike an + * unbracketed `WriteLiteral` call, whose argument is written straight to the response. + * + * Because a basic block cannot contain a branch, requiring `beginCall`, `writeLiteral`, and + * `endCall` to appear (in that order) in the same basic block, with no other + * `Begin`/`EndWriteTagHelperAttribute` call strictly between `beginCall` and `endCall`, + * guarantees that `beginCall`/`endCall` are the immediately enclosing bracket around + * `writeLiteral` on every path that reaches it (that is, the bracket opened by `beginCall` is + * still open, and not yet closed by some other `endCall`, at the point `writeLiteral` executes). + * No other such call can coincide with `writeLiteral` itself, so checking the whole open interval + * between `beginCall` and `endCall` is equivalent to checking it on both sides of `writeLiteral` + * separately. + * + * `beginCall`, `writeLiteral`, and `endCall` are additionally required to have an implicit `this` + * qualifier, which is how the Razor source generator always emits these calls. Combined with all + * three calls being required to lie in the same basic block (and hence the same method body), + * this ensures all three calls act on the same page instance, so a bracket on one page cannot be + * mistaken for a bracket around a `WriteLiteral` call on a different page. + * + * The `WriteLiteral`/`BeginWriteTagHelperAttribute`/`EndWriteTagHelperAttribute` methods are + * looked up via `any(MicrosoftAspNetCoreMvcRazorPageBase page).get...Method()` rather than + * through a single shared `page` variable bound across `writeLiteral`, `beginCall`, and + * `endCall`. These methods are inherited (not overridden) from the shared `RazorPageBase` + * framework type, so every generated Razor page class resolves to the same handful of `Method` + * entities; binding a single `page` variable across all three calls would force the join to be + * repeated once per generated page class in the codebase, rather than once per distinct `Method`, + * causing severe performance degradation on codebases with many Razor views. + */ +private predicate isBracketedForTagHelperAttribute(Call writeLiteral) { + exists(MethodCall beginCall, MethodCall endCall, BasicBlock bb, int i, int j, int k | + bb = writeLiteral.getBasicBlock() and + writeLiteral = any(MicrosoftAspNetCoreMvcRazorPageBase page).getWriteLiteralMethod().getACall() and + beginCall = + any(MicrosoftAspNetCoreMvcRazorPageBase page) + .getBeginWriteTagHelperAttributeMethod() + .getACall() and + endCall = + any(MicrosoftAspNetCoreMvcRazorPageBase page).getEndWriteTagHelperAttributeMethod().getACall() and + writeLiteral.(QualifiableExpr).hasImplicitThisQualifier() and + beginCall.hasImplicitThisQualifier() and + endCall.hasImplicitThisQualifier() and + bb.getNode(i) = beginCall.getControlFlowNode() and + bb.getNode(j) = writeLiteral.getControlFlowNode() and + bb.getNode(k) = endCall.getControlFlowNode() and + i < j and + j < k and + not exists(int l, Call other | + ( + other = + any(MicrosoftAspNetCoreMvcRazorPageBase page) + .getBeginWriteTagHelperAttributeMethod() + .getACall() or + other = + any(MicrosoftAspNetCoreMvcRazorPageBase page) + .getEndWriteTagHelperAttributeMethod() + .getACall() + ) and + bb.getNode(l) = other.getControlFlowNode() and + i < l and + l < k + ) + ) +} + /** * An expression that is used as an argument to `Page.WriteLiteral` in ASP.NET 6.0 razor page, typically in * a `.cshtml` file. + * + * `WriteLiteral` calls whose argument is captured for a tag helper attribute value (see + * `isBracketedForTagHelperAttribute`) are excluded, since such calls buffer the value as a tag + * helper attribute rather than writing it directly to the response. */ class MicrosoftAspNetRazorPageWriteLiteralSink extends AspNetCoreHtmlSink { MicrosoftAspNetRazorPageWriteLiteralSink() { - this.getExpr() = - any(MicrosoftAspNetCoreMvcRazorPageBase h).getWriteLiteralMethod().getACall().getAnArgument() + exists(Call writeLiteral | + writeLiteral = any(MicrosoftAspNetCoreMvcRazorPageBase h).getWriteLiteralMethod().getACall() and + this.getExpr() = writeLiteral.getAnArgument() and + not isBracketedForTagHelperAttribute(writeLiteral) + ) } } diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/RazorTagHelperAttribute.cshtml b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/RazorTagHelperAttribute.cshtml new file mode 100644 index 000000000000..8b1a393741c9 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/RazorTagHelperAttribute.cshtml @@ -0,0 +1 @@ +// empty diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/RazorTagHelperAttribute.cshtml.g.cs b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/RazorTagHelperAttribute.cshtml.g.cs new file mode 100644 index 000000000000..12c2b389709a --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/RazorTagHelperAttribute.cshtml.g.cs @@ -0,0 +1,89 @@ +// A hand-written test file that mimics the output of compiling a `.cshtml` file that has an +// element with a tag helper (e.g. `asp-for`) and an HTML attribute whose value contains a +// tainted expression, for example: +#pragma checksum "RazorTagHelperAttribute.cshtml" "{ff1816ec-aa5e-4d10-87f7-6f4963833460}" "c4ae76542f1958092cebd8f57beef899d20fc548" +// +#pragma warning disable 1591 +[assembly: global::Microsoft.AspNetCore.Razor.Hosting.RazorCompiledItemAttribute(typeof(dotnetweb.Pages.Pages_RazorTagHelperAttribute), @"mvc.1.0.razor-page", @"RazorTagHelperAttribute.cshtml")] +namespace dotnetweb.Pages +{ + #line hidden + using System; + using System.Collections.Generic; + using System.Linq; + using System.Threading.Tasks; + using Microsoft.AspNetCore.Mvc; + using Microsoft.AspNetCore.Mvc.Rendering; + using Microsoft.AspNetCore.Mvc.ViewFeatures; +#nullable restore +using dotnetweb; + +#line default +#line hidden +#nullable disable + [global::Microsoft.AspNetCore.Razor.Hosting.RazorSourceChecksumAttribute(@"SHA1", @"c4ae76542f1958092cebd8f57beef899d20fc548", @"RazorTagHelperAttribute.cshtml")] + public class Pages_RazorTagHelperAttribute : global::Microsoft.AspNetCore.Mvc.RazorPages.Page + { + #pragma warning disable 1998 + public async override global::System.Threading.Tasks.Task ExecuteAsync() + { +#nullable restore +#line 3 "RazorTagHelperAttribute.cshtml" + + var model = Request.Query["m"]; // $ Source=model + +#line default +#line hidden +#nullable disable + WriteLiteral(""); + + // BAD: a `WriteLiteral` call that is not bracketed by + // `BeginWriteTagHelperAttribute()`/`EndWriteTagHelperAttribute()` writes directly, + // unencoded, to the response and must still be flagged. + WriteLiteral(model); // $ Alert=model + + // GOOD: two independent tag helper attribute brackets that occur one after another + // in the same basic block must each be matched with their own nearest + // `Begin`/`EndWriteTagHelperAttribute` call, and neither should leak into the other. + BeginWriteTagHelperAttribute(); + WriteLiteral("literal-prefix-"); + var __tagHelperAttribute_2 = EndWriteTagHelperAttribute(); + BeginWriteTagHelperAttribute(); + WriteLiteral(model); + var __tagHelperAttribute_3 = EndWriteTagHelperAttribute(); + + // BAD: a bare `WriteLiteral` call sandwiched between two unrelated tag helper + // attribute brackets, in the same basic block, must not be mistaken for being + // captured by either neighboring bracket. + BeginWriteTagHelperAttribute(); + WriteLiteral("prefix"); + var __tagHelperAttribute_4 = EndWriteTagHelperAttribute(); + WriteLiteral(model); // $ Alert=model + BeginWriteTagHelperAttribute(); + WriteLiteral("suffix"); + var __tagHelperAttribute_5 = EndWriteTagHelperAttribute(); + } + #pragma warning restore 1998 + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.ViewFeatures.IModelExpressionProvider ModelExpressionProvider { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.IUrlHelper Url { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.IViewComponentHelper Component { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.Rendering.IJsonHelper Json { get; private set; } + [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute] + public global::Microsoft.AspNetCore.Mvc.Rendering.IHtmlHelper Html { get; private set; } + public global::Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary ViewData => (global::Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary)PageContext?.ViewData; + } +} +#pragma warning restore 1591 diff --git a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.expected b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.expected index fe184fdff75a..f560bb101456 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.expected +++ b/csharp/ql/test/query-tests/Security Features/CWE-079/XSS/XSS.expected @@ -1,5 +1,7 @@ #select | 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 | +| 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 | +| 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 | | 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 | | 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 | | 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 @@ edges | Index.cshtml:5:9:5:15 | access to local variable message : StringValues | Index.cshtml:14:16:14:22 | call to operator implicit conversion | provenance | | | Index.cshtml:5:19:5:31 | access to property Query : IQueryCollection | Index.cshtml:5:9:5:15 | access to local variable message : StringValues | provenance | | +| 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 | | +| 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 | | +| RazorTagHelperAttribute.cshtml:4:17:4:29 | access to property Query : IQueryCollection | RazorTagHelperAttribute.cshtml:4:9:4:13 | access to local variable model : StringValues | provenance | | | XSSAspNet.cs:19:17:19:21 | access to local variable sayHi : String | XSSAspNet.cs:26:30:26:34 | access to local variable sayHi | provenance | | | XSSAspNet.cs:19:17:19:21 | access to local variable sayHi : String | XSSAspNet.cs:36:40:36:44 | access to local variable sayHi | provenance | | | 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 | Index.cshtml:5:9:5:15 | access to local variable message : StringValues | semmle.label | access to local variable message : StringValues | | Index.cshtml:5:19:5:31 | access to property Query : IQueryCollection | semmle.label | access to property Query : IQueryCollection | | Index.cshtml:14:16:14:22 | call to operator implicit conversion | semmle.label | call to operator implicit conversion | +| RazorTagHelperAttribute.cshtml.g.cs:52:26:52:30 | call to operator implicit conversion | semmle.label | call to operator implicit conversion | +| RazorTagHelperAttribute.cshtml.g.cs:70:26:70:30 | call to operator implicit conversion | semmle.label | call to operator implicit conversion | +| RazorTagHelperAttribute.cshtml:4:9:4:13 | access to local variable model : StringValues | semmle.label | access to local variable model : StringValues | +| RazorTagHelperAttribute.cshtml:4:17:4:29 | access to property Query : IQueryCollection | semmle.label | access to property Query : IQueryCollection | | XSSAspNet.cs:19:17:19:21 | access to local variable sayHi : String | semmle.label | access to local variable sayHi : String | | XSSAspNet.cs:19:25:19:43 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection | | XSSAspNet.cs:19:25:19:52 | access to indexer : String | semmle.label | access to indexer : String |