Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +187 to +190
*
* 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)
)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// empty
Original file line number Diff line number Diff line change
@@ -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: <input asp-for="Value" value="@Model.Value">
#pragma checksum "RazorTagHelperAttribute.cshtml" "{ff1816ec-aa5e-4d10-87f7-6f4963833460}" "c4ae76542f1958092cebd8f57beef899d20fc548"
// <auto-generated/>
#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("<input type=\"text\" ");
// GOOD: this `WriteLiteral` call is generated for the value of an HTML attribute on
// an element with a tag helper (e.g. `asp-for`). It writes into an internal string
// buffer via `BeginWriteTagHelperAttribute()`/`EndWriteTagHelperAttribute()`, and the
// buffered text is HTML-attribute-encoded later, when the tag helper attribute value
// is rendered. It is not a direct, unencoded write, so it must not be flagged.
BeginWriteTagHelperAttribute();
WriteLiteral(model);
var __tagHelperAttribute_1 = EndWriteTagHelperAttribute();
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<string> Html { get; private set; }
public global::Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary<string> ViewData => (global::Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary<string>)PageContext?.ViewData;
}
}
#pragma warning restore 1591
Original file line number Diff line number Diff line change
@@ -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 |
Expand All @@ -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 | |
Expand Down Expand Up @@ -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 |
Expand Down
Loading