Conversation
Razor's source generator brackets WriteLiteral calls that populate an HTML attribute value on a tag-helper-enabled element (e.g. �sp-for) between BeginWriteTagHelperAttribute()/EndWriteTagHelperAttribute() calls. The captured text is buffered internally and HTML-attribute- encoded before being rendered, so it is not a real XSS sink, but cs/web/xss previously flagged it as one. - Add getBeginWriteTagHelperAttributeMethod() / getEndWriteTagHelperAttributeMethod() to MicrosoftAspNetCoreMvcRazorPageBase. - Add isBracketedForTagHelperAttribute() to Html.qll and use it to exclude bracketed WriteLiteral calls from MicrosoftAspNetRazorPageWriteLiteralSink, using same-basic-block, immediately-adjacent-bracket matching so unrelated bracket pairs cannot "adopt" an unbracketed call. - Add RazorTagHelperAttribute.cshtml(.g.cs) test coverage: a suppressed bracketed write, an unbracketed positive control, two independent brackets in one basic block, and a bare write sandwiched between brackets. - Add change note (majorAnalysis). Diagnosed and validated against the real customer reproduction that motivated this fix (field-security-codeql#257 / github#261): the false positive (ChangeAccountInfo.cshtml) is no longer reported, while the genuine Html.Raw-based positive control (ProfileSummary.cshtml) remains reported. Full CWE-079 test suite (6 tests) passes with no regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot Code Review correctly flagged that isBracketedForTagHelperAttribute never related the receivers of beginCall/writeLiteral/endCall, so a bracket on one page instance could theoretically be mistaken for a bracket around a WriteLiteral call on a different page (e.g. otherPage.BeginWriteTagHelperAttribute(); this.WriteLiteral(model); otherPage.EndWriteTagHelperAttribute();). Require all three calls to have an implicit his qualifier, which is how the Razor source generator always emits them, guaranteeing they act on the same page instance. Re-verified: XSS.ql compiles, all 6 CWE-079 tests pass, and the real customer reproduction database still shows the FP suppressed and the genuine Html.Raw positive control still reported. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Shortened to a single terse sentence, matching the depth/style of other recent change-notes (one bullet, no implementation detail), and called out the ASP.NET Core Razor Pages/MVC scope. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The documentation incorrectly promises downstream HTML encoding instead of stating that the call only writes to a temporary buffer.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Prevents false-positive C# XSS alerts for buffered Razor tag-helper attribute writes.
Changes:
- Models Razor tag-helper attribute buffering methods.
- Excludes correctly bracketed
WriteLiteralcalls from direct XSS sinks. - Adds regression fixtures and a change note.
| File | Description |
|---|---|
Html.qll |
Detects buffered WriteLiteral calls. |
AspNetCore.qll |
Models tag-helper buffering methods. |
RazorTagHelperAttribute.cshtml |
Provides source-map fixture. |
RazorTagHelperAttribute.cshtml.g.cs |
Adds generated-code test scenarios. |
XSS.expected |
Updates expected query results. |
| Change note | Documents the analysis change. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+187
to
+190
| * 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. |
| --- | ||
| 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cs/web/xsscurrently flagsRazorPageBase.WriteLiteral(...)calls that the Razor source generator emits for the value of an HTML attribute on an element that also carries a tag helper (for example,asp-for="Model.Something"). This is a false positive: the Razor codegen brackets these calls betweenBeginWriteTagHelperAttribute()/EndWriteTagHelperAttribute(), buffers the text internally, and HTML-attribute-encodes it before it is ever written to the response. It is not a real XSS sink.Example
Given a controller action that binds user-provided input to a view model:
and a Razor Pages/MVC view that renders one of its properties through a tag helper attribute:
cs/web/xsspreviously reported:This is a false positive. The value assigned to the
asp-forattribute is captured into an internal string buffer via matchingBeginWriteTagHelperAttribute()/EndWriteTagHelperAttribute()calls generated around theWriteLiteralcall, and that buffered value is HTML-attribute-encoded before the tag helper's output is rendered, so it never reaches the response unencoded.Fix
AspNetCore.qll: addgetBeginWriteTagHelperAttributeMethod()/getEndWriteTagHelperAttributeMethod()toMicrosoftAspNetCoreMvcRazorPageBase.Html.qll: addisBracketedForTagHelperAttribute()and use it to exclude bracketedWriteLiteralcalls fromMicrosoftAspNetRazorPageWriteLiteralSink. The predicate requiresbeginCall,writeLiteral,endCallto appear (in that order) in the same basic block, with no otherBegin/EndWriteTagHelperAttributecall in between on either side, and all three calls to share an implicitthisreceiver, so an unrelated bracket (on this or another page instance) can't "adopt" an unbracketed call.Test coverage
Added
RazorTagHelperAttribute.cshtml/.cshtml.g.csto the existingSecurity Features/CWE-079/XSStest, covering:WriteLiteral(model)(must not alert, suppressed FP).WriteLiteral(model)(must alert, positive control).WriteLiteral(model)sandwiched between two unrelated brackets (must still alert).Full
Security Features/CWE-079suite (6 tests) passes with no regressions. Also validated end-to-end against a small standalone ASP.NET Core Razor Pages reproduction project mirroring the example above: the false positive is no longer reported, while a genuineHtml.Raw-based positive control in the same project remains reported.Change note
Added
csharp/ql/lib/change-notes/2026-09-18-razor-tag-helper-attribute-xss-fp.md(category: majorAnalysis).