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..266ef985cd47 --- /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`) are HTML-attribute-encoded before being rendered, 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..2934c5c123de 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,83 @@ 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 on `page`, 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, unencoded, to the response: `WriteLiteral` + * appends to an internal string buffer, `EndWriteTagHelperAttribute()` returns that buffer, and + * the buffered text is subsequently stored as a tag helper attribute value and HTML-attribute- + * encoded when the tag helper's output is rendered. This is therefore not a real sink. + * + * 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 from `page` strictly between `beginCall` and + * `writeLiteral`, or between `writeLiteral` 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). + * + * `beginCall`, `writeLiteral`, and `endCall` are additionally required to have an implicit `this` + * qualifier, which is how the Razor source generator always emits these calls. 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. + */ +private predicate isBracketedForTagHelperAttribute(Call writeLiteral) { + exists( + MicrosoftAspNetCoreMvcRazorPageBase page, MethodCall beginCall, MethodCall endCall, int i, + int j, int k + | + writeLiteral = page.getWriteLiteralMethod().getACall() and + beginCall = page.getBeginWriteTagHelperAttributeMethod().getACall() and + endCall = page.getEndWriteTagHelperAttributeMethod().getACall() and + writeLiteral.(QualifiableExpr).hasImplicitThisQualifier() and + beginCall.hasImplicitThisQualifier() and + endCall.hasImplicitThisQualifier() and + writeLiteral.getBasicBlock().getNode(i) = beginCall.getControlFlowNode() and + writeLiteral.getBasicBlock().getNode(j) = writeLiteral.getControlFlowNode() and + writeLiteral.getBasicBlock().getNode(k) = endCall.getControlFlowNode() and + i < j and + j < k and + not exists(int i2, Call other | + ( + other = page.getBeginWriteTagHelperAttributeMethod().getACall() or + other = page.getEndWriteTagHelperAttributeMethod().getACall() + ) and + writeLiteral.getBasicBlock().getNode(i2) = other.getControlFlowNode() and + i < i2 and + i2 < j + ) and + not exists(int k2, Call other | + ( + other = page.getBeginWriteTagHelperAttributeMethod().getACall() or + other = page.getEndWriteTagHelperAttributeMethod().getACall() + ) and + writeLiteral.getBasicBlock().getNode(k2) = other.getControlFlowNode() and + j < k2 and + k2 < 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 values are HTML-attribute-encoded + * later and are not written unencoded 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 |