diff --git a/csharp/ql/integration-tests/all-platforms/blazor_net_8/XSS.expected b/csharp/ql/integration-tests/all-platforms/blazor_net_8/XSS.expected index 204c31945956..dbf056053b30 100644 --- a/csharp/ql/integration-tests/all-platforms/blazor_net_8/XSS.expected +++ b/csharp/ql/integration-tests/all-platforms/blazor_net_8/XSS.expected @@ -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 | 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 | 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); ; 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 | semmle.label | call to method TypeCheck : String | subpaths diff --git a/csharp/ql/lib/change-notes/2025-03-08-blazor-parameter-passing-string-literal.md b/csharp/ql/lib/change-notes/2025-03-08-blazor-parameter-passing-string-literal.md new file mode 100644 index 000000000000..66ebd26f653d --- /dev/null +++ b/csharp/ql/lib/change-notes/2025-03-08-blazor-parameter-passing-string-literal.md @@ -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. \ No newline at end of file diff --git a/csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/aspnetcore/Components.qll b/csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/aspnetcore/Components.qll index be937661b477..1fa427f4d19d 100644 --- a/csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/aspnetcore/Components.qll +++ b/csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/aspnetcore/Components.qll @@ -122,6 +122,38 @@ private class MicrosoftAspNetCoreComponentsAddComponentParameterMethod extends M } } +/** + * The `Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder::OpenComponent` 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 @@ -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 @@ -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] + ) + ) } /** diff --git a/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/NameList2.cs b/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/NameList2.cs new file mode 100644 index 000000000000..d27d6f2dcde9 --- /dev/null +++ b/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/NameList2.cs @@ -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(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(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 Names { get; set; } = new List(); + } +} \ No newline at end of file diff --git a/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/Xss.expected b/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/Xss.expected index 951269f2b580..3b9a54fafb3d 100644 --- a/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/Xss.expected +++ b/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/Xss.expected @@ -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 | diff --git a/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/remoteFlowSource.expected b/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/remoteFlowSource.expected index 2a9268cf01e3..e1b3724f44d6 100644 --- a/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/remoteFlowSource.expected +++ b/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/remoteFlowSource.expected @@ -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 |