Merge pull request #19145 from tamasvajk/tamasvajk/blazor/parameter-passing-jumpnode-2

C#: Blazor: Support string literals as property names in jump nodes
2026-04-26 09:15:12 +02:00 · 2025-04-03 10:07:59 +02:00
parent 5c42c0ba4c a570a728bd
commit befc2fd7c1
6 changed files with 150 additions and 1 deletions
--- 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> : 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
--- 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
@@ -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.
--- 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<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

@@ -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]
+        )
+      )
    }

    /**
--- a/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/NameList2.cs
+++ 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<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>();
+    }
+}
--- 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 |
--- 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 |