Skip to content

C#: Blazor: Support string literals as property names in jump nodes #19145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
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
@@ -1,8 +1,18 @@
#select
| BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | $@ flows to here and is written to HTML or JavaScript. | BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | User-provided value |
| BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | $@ flows to here and is written to HTML or JavaScript. | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | User-provided value |
| BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | $@ flows to here and is written to HTML or JavaScript. | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | User-provided value |
edges
| BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | BlazorTest/obj/Debug/net8.0/generated/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:497:59:505:13 | call to method TypeCheck<String> : String | provenance | Src:MaD:2 MaD:3 |
| BlazorTest/obj/Debug/net8.0/generated/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:497:59:505:13 | call to method TypeCheck<String> : String | BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | provenance | Sink:MaD:1 |
models
| 1 | Sink: Microsoft.AspNetCore.Components; MarkupString; false; MarkupString; (System.String); ; Argument[0]; html-injection; manual |
| 2 | Source: Microsoft.AspNetCore.Components; SupplyParameterFromQueryAttribute; false; ; ; Attribute.Getter; ReturnValue; remote; manual |
| 3 | Summary: Microsoft.AspNetCore.Components.CompilerServices; RuntimeHelpers; false; TypeCheck<T>; (T); ; Argument[0]; ReturnValue; value; manual |
nodes
| BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | semmle.label | access to property Value |
| BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | semmle.label | access to property UrlParam |
| BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | semmle.label | access to property QueryParam |
| BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | semmle.label | access to property QueryParam : String |
| BlazorTest/obj/Debug/net8.0/generated/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:497:59:505:13 | call to method TypeCheck<String> : String | semmle.label | call to method TypeCheck<String> : String |
subpaths
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Blazor support can now better recognize when a property being set is specified with a string literal, rather than referenced in a `nameof` expression.
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,38 @@ private class MicrosoftAspNetCoreComponentsAddComponentParameterMethod extends M
}
}

/**
* The `Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder::OpenComponent<TComponent>` method.
*/
private class MicrosoftAspNetCoreComponentsOpenComponentTComponentMethod extends Method {
MicrosoftAspNetCoreComponentsOpenComponentTComponentMethod() {
this.hasFullyQualifiedName("Microsoft.AspNetCore.Components.Rendering", "RenderTreeBuilder",
"OpenComponent`1") and
this.getNumberOfParameters() = 1
}
}

/**
* The `Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder::OpenComponent` method.
*/
private class MicrosoftAspNetCoreComponentsOpenComponentMethod extends Method {
MicrosoftAspNetCoreComponentsOpenComponentMethod() {
this.hasFullyQualifiedName("Microsoft.AspNetCore.Components.Rendering", "RenderTreeBuilder",
"OpenComponent") and
this.getNumberOfParameters() = 2
}
}

/**
* The `Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder::CloseComponent` method.
*/
private class MicrosoftAspNetCoreComponentsCloseComponentMethod extends Method {
MicrosoftAspNetCoreComponentsCloseComponentMethod() {
this.hasFullyQualifiedName("Microsoft.AspNetCore.Components.Rendering", "RenderTreeBuilder",
"CloseComponent")
}
}

private module Sources {
private import semmle.code.csharp.security.dataflow.flowsources.Remote

Expand All @@ -144,6 +176,37 @@ private module Sources {
}
}

/**
* Holds for matching `RenderTreeBuilder.OpenComponent` and `RenderTreeBuilder.CloseComponent` calls with index `openCallIndex` and `closeCallIndex` respectively
* within the `enclosing` enclosing callabale. The `componentType` is the type of the component that is being opened and closed.
*/
private predicate matchingOpenCloseComponentCalls(
MethodCall openCall, int openCallIndex, MethodCall closeCall, int closeCallIndex,
Callable enclosing, Type componentType
) {
(
openCall.getTarget().getUnboundDeclaration() instanceof
MicrosoftAspNetCoreComponentsOpenComponentTComponentMethod and
openCall.getTarget().(ConstructedGeneric).getTypeArgument(0) = componentType
or
openCall.getTarget() instanceof MicrosoftAspNetCoreComponentsOpenComponentMethod and
openCall.getArgument(1).(TypeofExpr).getTypeAccess().getTarget() = componentType
) and
openCall.getEnclosingCallable() = enclosing and
closeCall.getTarget() instanceof MicrosoftAspNetCoreComponentsCloseComponentMethod and
closeCall.getEnclosingCallable() = enclosing and
exists(BlockStmt block |
block = closeCall.getParent().getParent() and
block = openCall.getParent().getParent() and
block.getChildStmt(openCallIndex) = openCall.getParent() and
closeCallIndex =
min(int closeCallIndex0 |
block.getChildStmt(closeCallIndex0) = closeCall.getParent() and
closeCallIndex0 > openCallIndex
)
)
}

private module JumpNodes {
/**
* A call to `Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder::AddComponentParameter` which
Expand All @@ -159,7 +222,23 @@ private module JumpNodes {
*/
Property getParameterProperty() {
result.getAnAttribute() instanceof MicrosoftAspNetCoreComponentsParameterAttribute and
exists(NameOfExpr ne | ne = this.getArgument(1) | result.getAnAccess() = ne.getAccess())
(
exists(NameOfExpr ne | ne = this.getArgument(1) | result.getAnAccess() = ne.getAccess())
or
exists(
string propertyName, MethodCall openComponent, BlockStmt block, int openIdx, int closeIdx,
int thisIdx
|
propertyName = this.getArgument(1).(StringLiteral).getValue() and
result.hasName(propertyName) and
matchingOpenCloseComponentCalls(openComponent, openIdx, _, closeIdx,
this.getEnclosingCallable(), result.getDeclaringType()) and
block = this.getParent().getParent() and
block = openComponent.getParent().getParent() and
block.getChildStmt(thisIdx) = this.getParent() and
thisIdx in [openIdx + 1 .. closeIdx - 1]
)
)
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
namespace VulnerableBlazorApp.Components
{
using System.Collections.Generic;
using Microsoft.AspNetCore.Components;

[RouteAttribute("/names2/{name?}")]
public partial class NameList2 : Microsoft.AspNetCore.Components.ComponentBase
{
protected override void BuildRenderTree(Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder builder)
{
if (Names is not null)
{
builder.OpenElement(0, "div");
builder.OpenElement(1, "ul");
foreach (var name in Names)
{
builder.OpenElement(2, "li");
builder.OpenComponent<VulnerableBlazorApp.Components.Name>(3);
builder.AddComponentParameter(4, "TheName", name);
builder.CloseComponent();
builder.CloseElement();
}
builder.CloseElement();
builder.CloseElement();
}

builder.OpenElement(5, "div");
builder.OpenElement(6, "p");
builder.AddContent(7, "Name: ");
builder.OpenComponent<VulnerableBlazorApp.Components.Name>(8);
builder.AddComponentParameter(9, "TheName", Name);
builder.CloseComponent();
builder.CloseElement();
}

[Parameter]
public string Name { get; set; }

protected override void OnParametersSet()
{
if (Name is not null)
{
Names.Add(Name);
}
}


public List<string> Names { get; set; } = new List<string>();
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
edges
| NameList2.cs:31:57:31:60 | access to property Name : String | Name.cs:13:53:13:59 | access to property TheName | provenance | Sink:MaD:149 |
| NameList.cs:31:99:31:102 | access to property Name : String | Name.cs:13:53:13:59 | access to property TheName | provenance | Sink:MaD:149 |
nodes
| Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | semmle.label | access to property UrlParam |
| Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | semmle.label | access to property QueryParam |
| Name.cs:13:53:13:59 | access to property TheName | semmle.label | access to property TheName |
| NameList2.cs:31:57:31:60 | access to property Name : String | semmle.label | access to property Name : String |
| NameList.cs:31:99:31:102 | access to property Name : String | semmle.label | access to property Name : String |
subpaths
#select
| Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | $@ flows to here and is written to HTML or JavaScript. | Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | User-provided value |
| Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | $@ flows to here and is written to HTML or JavaScript. | Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | User-provided value |
| Name.cs:13:53:13:59 | access to property TheName | NameList2.cs:31:57:31:60 | access to property Name : String | Name.cs:13:53:13:59 | access to property TheName | $@ flows to here and is written to HTML or JavaScript. | NameList2.cs:31:57:31:60 | access to property Name : String | User-provided value |
| Name.cs:13:53:13:59 | access to property TheName | NameList.cs:31:99:31:102 | access to property Name : String | Name.cs:13:53:13:59 | access to property TheName | $@ flows to here and is written to HTML or JavaScript. | NameList.cs:31:99:31:102 | access to property Name : String | User-provided value |
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
| Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | ASP.NET Core component route parameter |
| Components_Pages_TestPage_razor.g.cs:176:1:176:10 | access to property QueryParam | external |
| Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | external |
| NameList2.cs:31:57:31:60 | access to property Name | ASP.NET Core component route parameter |
| NameList2.cs:41:17:41:20 | access to property Name | ASP.NET Core component route parameter |
| NameList2.cs:43:27:43:30 | access to property Name | ASP.NET Core component route parameter |
| NameList.cs:31:99:31:102 | access to property Name | ASP.NET Core component route parameter |
| NameList.cs:41:17:41:20 | access to property Name | ASP.NET Core component route parameter |
| NameList.cs:43:27:43:30 | access to property Name | ASP.NET Core component route parameter |