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

tamasvajk · web-flow · commit 342d4a698220 · 2025-03-28T16:03:54.000+01:00
C#: Blazor: Add non-local jump node for parameter passing
diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/SourceGenerators/DotnetSourceGeneratorWrapper/DotnetSourceGeneratorWrapper.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/SourceGenerators/DotnetSourceGeneratorWrapper/DotnetSourceGeneratorWrapper.cs
@@ -37,7 +37,8 @@ public IEnumerable<string> RunSourceGenerator(IEnumerable<string> additionalFile
         {
             try
             {
-                var relativePathToCsProj = Path.GetRelativePath(sourceDir, csprojFile);
+                var relativePathToCsProj = Path.GetRelativePath(sourceDir, csprojFile)
+                    .Replace('\\', '/'); // Ensure we're generating the same hash regardless of the OS
                 var name = FileUtils.ComputeHash($"{relativePathToCsProj}\n{this.GetType().Name}");
                 using var tempDir = new TemporaryDirectory(Path.Join(FileUtils.GetTemporaryWorkingDirectory(out _), "source-generator"), "source generator temporary", logger);
                 var analyzerConfigPath = Path.Combine(tempDir.DirInfo.FullName, $"{name}.txt");
diff --git a/csharp/ql/integration-tests/all-platforms/blazor/BlazorTest/Components/Pages/TestPage.razor b/csharp/ql/integration-tests/all-platforms/blazor/BlazorTest/Components/Pages/TestPage.razor
@@ -81,6 +81,10 @@
     <MyOutput Value="@InputValue6" />
 </div>
 
+<div>
+    <MyOutput Value="@QueryParam" />
+</div>
+
 @code {
 
     public class Container
diff --git a/csharp/ql/integration-tests/all-platforms/blazor/XSS.expected b/csharp/ql/integration-tests/all-platforms/blazor/XSS.expected
@@ -0,0 +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/net9.0/generated/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577:13 | call to method TypeCheck<String> : String | provenance | Src:MaD:2 MaD:3 |
+| BlazorTest/obj/Debug/net9.0/generated/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577: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/net9.0/generated/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577:13 | call to method TypeCheck<String> : String | semmle.label | call to method TypeCheck<String> : String |
+subpaths
diff --git a/csharp/ql/integration-tests/all-platforms/blazor/XSS.qlref b/csharp/ql/integration-tests/all-platforms/blazor/XSS.qlref
@@ -0,0 +1,2 @@
+query: Security Features/CWE-079/XSS.ql
+postprocess: utils/test/PrettyPrintModels.ql
diff --git a/csharp/ql/integration-tests/all-platforms/blazor_build_mode_none/BlazorTest/Components/Pages/TestPage.razor b/csharp/ql/integration-tests/all-platforms/blazor_build_mode_none/BlazorTest/Components/Pages/TestPage.razor
@@ -81,6 +81,10 @@
     <MyOutput Value="@InputValue6" />
 </div>
 
+<div>
+    <MyOutput Value="@QueryParam" />
+</div>
+
 @code {
 
     public class Container
diff --git a/csharp/ql/integration-tests/all-platforms/blazor_build_mode_none/XSS.expected b/csharp/ql/integration-tests/all-platforms/blazor_build_mode_none/XSS.expected
@@ -0,0 +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 | test-db/working/razor/AC613014E59A413B9538FF8068364499/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577:13 | call to method TypeCheck<String> : String | provenance | Src:MaD:2 MaD:3 |
+| test-db/working/razor/AC613014E59A413B9538FF8068364499/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577: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 |
+| test-db/working/razor/AC613014E59A413B9538FF8068364499/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577:13 | call to method TypeCheck<String> : String | semmle.label | call to method TypeCheck<String> : String |
+subpaths
diff --git a/csharp/ql/integration-tests/all-platforms/blazor_build_mode_none/XSS.qlref b/csharp/ql/integration-tests/all-platforms/blazor_build_mode_none/XSS.qlref
@@ -0,0 +1,2 @@
+query: Security Features/CWE-079/XSS.ql
+postprocess: utils/test/PrettyPrintModels.ql
diff --git a/csharp/ql/integration-tests/all-platforms/blazor_net_8/BlazorTest/Components/Pages/TestPage.razor b/csharp/ql/integration-tests/all-platforms/blazor_net_8/BlazorTest/Components/Pages/TestPage.razor
@@ -81,6 +81,10 @@
     <MyOutput Value="@InputValue6" />
 </div>
 
+<div>
+    <MyOutput Value="@QueryParam" />
+</div>
+
 @code {
 
     public class Container
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
@@ -0,0 +1,8 @@
+#select
+| 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
+nodes
+| 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 |
+subpaths
diff --git a/csharp/ql/integration-tests/all-platforms/blazor_net_8/XSS.qlref b/csharp/ql/integration-tests/all-platforms/blazor_net_8/XSS.qlref
@@ -0,0 +1,2 @@
+query: Security Features/CWE-079/XSS.ql
+postprocess: utils/test/PrettyPrintModels.ql
diff --git a/csharp/ql/lib/change-notes/2025-03-26-blazor-parameter-passing.md b/csharp/ql/lib/change-notes/2025-03-26-blazor-parameter-passing.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Modeled parameter passing between Blazor parent and child components.
diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
@@ -147,6 +147,16 @@ predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) }
 pragma[inline]
 predicate localExprFlow(Expr e1, Expr e2) { localFlow(exprNode(e1), exprNode(e2)) }
 
+/**
+ * A module importing the modules that provide non local jump node declarations,
+ * ensuring that they are visible to the taint tracking / data flow library.
+ */
+private module JumpNodes {
+  private import semmle.code.csharp.frameworks.microsoft.aspnetcore.Components
+  private import semmle.code.csharp.frameworks.Razor
+  private import semmle.code.csharp.frameworks.NHibernate
+}
+
 /**
  * A data flow node that jumps between callables. This can be extended in
  * framework code to add additional data flow steps.
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
@@ -112,6 +112,16 @@ class MicrosoftAspNetCoreComponentsComponent extends Class {
   }
 }
 
+/**
+ * The `Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder::AddComponentParameter` method.
+ */
+private class MicrosoftAspNetCoreComponentsAddComponentParameterMethod extends Method {
+  MicrosoftAspNetCoreComponentsAddComponentParameterMethod() {
+    this.hasFullyQualifiedName("Microsoft.AspNetCore.Components.Rendering", "RenderTreeBuilder",
+      "AddComponentParameter")
+  }
+}
+
 private module Sources {
   private import semmle.code.csharp.security.dataflow.flowsources.Remote
 
@@ -133,3 +143,44 @@ private module Sources {
     override string getSourceType() { result = "ASP.NET Core component route parameter" }
   }
 }
+
+private module JumpNodes {
+  /**
+   * A call to `Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder::AddComponentParameter` which
+   * sets the value of a parameter.
+   */
+  private class ParameterPassingCall extends Call {
+    ParameterPassingCall() {
+      this.getTarget() instanceof MicrosoftAspNetCoreComponentsAddComponentParameterMethod
+    }
+
+    /**
+     * Gets the property whose value is being set.
+     */
+    Property getParameterProperty() {
+      result.getAnAttribute() instanceof MicrosoftAspNetCoreComponentsParameterAttribute and
+      exists(NameOfExpr ne | ne = this.getArgument(1) | result.getAnAccess() = ne.getAccess())
+    }
+
+    /**
+     * Gets the value being set.
+     */
+    Expr getParameterValue() { result = this.getArgument(2) }
+  }
+
+  private class ComponentParameterJump extends DataFlow::NonLocalJumpNode {
+    Property prop;
+
+    ComponentParameterJump() {
+      exists(ParameterPassingCall call |
+        prop = call.getParameterProperty() and
+        this.asExpr() = call.getParameterValue()
+      )
+    }
+
+    override DataFlow::Node getAJumpSuccessor(boolean preservesValue) {
+      preservesValue = true and
+      result.asExpr() = prop.getAnAccess()
+    }
+  }
+}
diff --git a/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/Name.cs b/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/Name.cs
@@ -0,0 +1,22 @@
+namespace VulnerableBlazorApp.Components
+{
+    using Microsoft.AspNetCore.Components;
+
+    public partial class Name : Microsoft.AspNetCore.Components.ComponentBase
+    {
+        protected override void BuildRenderTree(Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder builder)
+        {
+            if (TheName is not null)
+            {
+                builder.OpenElement(0, "div");
+                builder.OpenElement(1, "p");
+                builder.AddContent(2, (MarkupString)TheName);
+                builder.CloseElement();
+                builder.CloseElement();
+            }
+        }
+
+        [Parameter]
+        public string TheName { get; set; }
+    }
+}
diff --git a/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/NameList.cs b/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/NameList.cs
@@ -0,0 +1,50 @@
+namespace VulnerableBlazorApp.Components
+{
+    using System.Collections.Generic;
+    using Microsoft.AspNetCore.Components;
+
+    [RouteAttribute("/names/{name?}")]
+    public partial class NameList : 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, nameof(VulnerableBlazorApp.Components.Name.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, nameof(VulnerableBlazorApp.Components.Name.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>();
+    }
+}
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
@@ -0,0 +1,12 @@
+edges
+| 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 |
+| 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 | 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/Xss.qlref b/csharp/ql/test/library-tests/frameworks/microsoft/aspnetcore/blazor/Xss.qlref
@@ -0,0 +1 @@
+Security Features/CWE-079/XSS.ql
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
@@ -2,3 +2,6 @@
 | 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 |
+| 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 |

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,8 @@ public IEnumerable<string> RunSourceGenerator(IEnumerable<string> additionalFile`
`37`	`37`	`{`
`38`	`38`	`try`
`39`	`39`	`{`
`40`		`- var relativePathToCsProj = Path.GetRelativePath(sourceDir, csprojFile);`
	`40`	`+ var relativePathToCsProj = Path.GetRelativePath(sourceDir, csprojFile)`
	`41`	`+ .Replace('\\', '/'); // Ensure we're generating the same hash regardless of the OS`
`41`	`42`	`var name = FileUtils.ComputeHash($"{relativePathToCsProj}\n{this.GetType().Name}");`
`42`	`43`	`using var tempDir = new TemporaryDirectory(Path.Join(FileUtils.GetTemporaryWorkingDirectory(out _), "source-generator"), "source generator temporary", logger);`
`43`	`44`	`var analyzerConfigPath = Path.Combine(tempDir.DirInfo.FullName, $"{name}.txt");`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+query: Security Features/CWE-079/XSS.ql`
	`2`	`+postprocess: utils/test/PrettyPrintModels.ql`