Merge pull request #15039 from joefarebrother/csharp-razor-flow-page-models

C#: Add flow steps from a PageModel to cshtml page.

Author: joefarebrother

csharp/ql/lib/semmle/code/csharp/frameworks/Razor.qll

@@ -4,6 +4,7 @@ private import csharp
 private import codeql.util.Unit
 private import codeql.util.FilePath
 private import semmle.code.csharp.frameworks.microsoft.AspNetCore
+private import semmle.code.csharp.dataflow.internal.DataFlowPrivate
 
 /** A call to the `View` method */
 private class ViewCall extends MethodCall {
@@ -215,3 +216,62 @@ private class RelativeViewCallFilepath extends NormalizableFilepath {
   /** Holds if this string is the `idx`th path that will be searched for the `vc` call. */
   predicate hasViewCallWithIndex(ViewCall vc, int idx) { vc = vc_ and idx = idx_ }
 }
+
+/** A subclass of `Microsoft.AspNetCore.Mvc.RazorPages.PageModel` */
+class PageModelClass extends Class {
+  PageModelClass() {
+    this.getABaseType+().hasFullyQualifiedName("Microsoft.AspNetCore.Mvc.RazorPages", "PageModel")
+  }
+
+  /** Gets a handler method such as `OnGetAsync` */
+  Method getAHandlerMethod() {
+    result = this.getAMethod() and
+    result.getName().matches("On%") and
+    not exists(Attribute attr |
+      attr = result.getAnAttribute() and
+      attr.getType()
+          .hasFullyQualifiedName("Microsoft.AspNetCore.Mvc.RazorPages", "NonHandlerAttribute")
+    )
+  }
+
+  /** Gets the Razor Page that has this PageModel. */
+  RazorViewClass getPage() {
+    exists(Property modelProp |
+      modelProp.hasName("Model") and
+      modelProp.getType() = this and
+      modelProp.getDeclaringType() = result
+    )
+  }
+}
+
+private MethodCall getAPageCall(PageModelClass pm) {
+  result.getEnclosingCallable() = pm.getAHandlerMethod() and
+  result
+      .getTarget()
+      .hasFullyQualifiedName("Microsoft.AspNetCore.Mvc.RazorPages", "PageModel",
+        ["Page", "RedirectToPage"])
+}
+
+private ThisAccess getThisCallInVoidHandler(PageModelClass pm) {
+  result.getEnclosingCallable() = pm.getAHandlerMethod() and
+  result.getEnclosingCallable().getReturnType() instanceof VoidType
+}
+
+private class PageModelJumpNode extends DataFlow::NonLocalJumpNode {
+  PageModelClass pm;
+
+  PageModelJumpNode() {
+    this.asExpr() = getAPageCall(pm).getQualifier()
+    or
+    this.(PostUpdateNode).getPreUpdateNode().asExpr() = getThisCallInVoidHandler(pm)
+  }
+
+  override DataFlow::Node getAJumpSuccessor(boolean preservesValue) {
+    preservesValue = true and
+    exists(PropertyAccess modelProp |
+      result.asExpr() = modelProp and
+      modelProp.getTarget().hasName("Model") and
+      modelProp.getEnclosingCallable().getDeclaringType() = pm.getPage()
+    )
+  }
+}

csharp/ql/src/change-notes/2023-12-12-page-model-flow-steps.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Modelled additional flow steps to track flow from handler methods of a `PageModel` class to the corresponding Razor Page (`.cshtml`) file, which may result in additional results for queries such as `cs/web/xss`.

csharp/ql/test/query-tests/Security Features/CWE-079/XssPageModels/TestModel.cs

@@ -0,0 +1,20 @@
+using Microsoft.AspNetCore.Mvc.RazorPages;
+using Microsoft.AspNetCore.Mvc;
+using System.Net;
+using System.Threading.Tasks;
+namespace test;
+
+class TestModel : PageModel {
+    public string Name { get; set; } = "abc";
+
+    private string source() { return "x"; }
+
+    public async Task<IActionResult> OnPostAsync() {
+        this.Name = source();
+        return Page();
+    }
+
+    public void OnGet() { 
+        Name = source();
+    }
+}

csharp/ql/test/query-tests/Security Features/CWE-079/XssPageModels/TestPage.cshtml

@@ -0,0 +1,5 @@
+@page
+@model TestModel
+@namespace test 
+
+<h3>Hello "@Html.Raw(Model.Name)"</h3>

csharp/ql/test/query-tests/Security Features/CWE-079/XssPageModels/TestPage.cshtml.g.cs

@@ -0,0 +1,65 @@
+// A hand-written test file that mimics the output of compiling a `.cshtml` file
+
+// <auto-generated/>
+#pragma warning disable 1591
+[assembly: global::Microsoft.AspNetCore.Razor.Hosting.RazorCompiledItemAttribute(typeof(test.Pages.Pages_TestPage), @"mvc.1.0.razor-page", @"TestPage.cshrml")]
+namespace test.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 test;
+
+#line default
+#line hidden
+#nullable disable
+    [global::Microsoft.AspNetCore.Razor.Hosting.RazorSourceChecksumAttribute(@"SHA1", @"c4ae76542f1958092cebd8f57beef899d20fc548", @"TestPage.cshtml")]
+    // [global::Microsoft.AspNetCore.Razor.Hosting.RazorSourceChecksumAttribute(@"SHA1", @"c13da96c2597d5ddb7d415fb4892c644a268f50b", @"/Pages/_ViewImports.cshtml")]
+    internal sealed class Pages_TestPage : global::Microsoft.AspNetCore.Mvc.RazorPages.Page
+    {
+        #pragma warning disable 1998
+        public async override global::System.Threading.Tasks.Task ExecuteAsync()
+        {
+#nullable disable
+            WriteLiteral("<h3>Hello \"");
+#nullable restore
+#line 5 "TestPage.cshtml"
+Write(Html.Raw(Model.Name));
+
+#line default
+#line hidden
+#nullable disable
+            WriteLiteral("\"</h3>\n");
+#nullable restore
+        }
+              #pragma warning restore 1998
+        #nullable restore
+        [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
+        public global::Microsoft.AspNetCore.Mvc.ViewFeatures.IModelExpressionProvider ModelExpressionProvider { get; private set; } = default!;
+        #nullable disable
+        #nullable restore
+        [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
+        public global::Microsoft.AspNetCore.Mvc.IUrlHelper Url { get; private set; } = default!;
+        #nullable disable
+        #nullable restore
+        [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
+        public global::Microsoft.AspNetCore.Mvc.IViewComponentHelper Component { get; private set; } = default!;
+        #nullable disable
+        #nullable restore
+        [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
+        public global::Microsoft.AspNetCore.Mvc.Rendering.IJsonHelper Json { get; private set; } = default!;
+        #nullable disable
+        #nullable restore
+        [global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
+        public global::Microsoft.AspNetCore.Mvc.Rendering.IHtmlHelper<TestModel> Html { get; private set; } = default!;
+        #nullable disable
+        public global::Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary<TestModel> ViewData => (global::Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary<TestModel>)PageContext?.ViewData;
+        public TestModel Model => ViewData.Model;    }
+}
+#pragma warning restore 1591

csharp/ql/test/query-tests/Security Features/CWE-079/XssPageModels/options

@@ -0,0 +1,3 @@
+semmle-extractor-options: /nostdlib /noconfig
+semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj
+semmle-extractor-options: --load-sources-from-project:../../../../resources/stubs/_frameworks/Microsoft.AspNetCore.App/Microsoft.AspNetCore.App.csproj

csharp/ql/test/query-tests/Security Features/CWE-079/XssPageModels/test.expected

@@ -0,0 +1,23 @@
+edges
+| TestModel.cs:13:9:13:12 | [post] this access : TestModel [property Name] : String | TestModel.cs:14:16:14:21 | this access : TestModel [property Name] : String |
+| TestModel.cs:13:21:13:28 | call to method source : String | TestModel.cs:13:9:13:12 | [post] this access : TestModel [property Name] : String |
+| TestModel.cs:14:16:14:21 | this access : TestModel [property Name] : String | TestPage.cshtml.g.cs:63:35:63:48 | access to property Model : TestModel [property Name] : String |
+| TestModel.cs:14:16:14:21 | this access : TestModel [property Name] : String | TestPage.cshtml:5:16:5:20 | access to property Model : TestModel [property Name] : String |
+| TestModel.cs:18:9:18:12 | [post] this access : TestModel [property Name] : String | TestPage.cshtml.g.cs:63:35:63:48 | access to property Model : TestModel [property Name] : String |
+| TestModel.cs:18:9:18:12 | [post] this access : TestModel [property Name] : String | TestPage.cshtml:5:16:5:20 | access to property Model : TestModel [property Name] : String |
+| TestModel.cs:18:16:18:23 | call to method source : String | TestModel.cs:18:9:18:12 | [post] this access : TestModel [property Name] : String |
+| TestPage.cshtml.g.cs:63:35:63:48 | access to property Model : TestModel [property Name] : String | TestPage.cshtml:5:16:5:20 | access to property Model : TestModel [property Name] : String |
+| TestPage.cshtml:5:16:5:20 | access to property Model : TestModel [property Name] : String | TestPage.cshtml:5:16:5:25 | access to property Name |
+nodes
+| TestModel.cs:13:9:13:12 | [post] this access : TestModel [property Name] : String | semmle.label | [post] this access : TestModel [property Name] : String |
+| TestModel.cs:13:21:13:28 | call to method source : String | semmle.label | call to method source : String |
+| TestModel.cs:14:16:14:21 | this access : TestModel [property Name] : String | semmle.label | this access : TestModel [property Name] : String |
+| TestModel.cs:18:9:18:12 | [post] this access : TestModel [property Name] : String | semmle.label | [post] this access : TestModel [property Name] : String |
+| TestModel.cs:18:16:18:23 | call to method source : String | semmle.label | call to method source : String |
+| TestPage.cshtml.g.cs:63:35:63:48 | access to property Model : TestModel [property Name] : String | semmle.label | access to property Model : TestModel [property Name] : String |
+| TestPage.cshtml:5:16:5:20 | access to property Model : TestModel [property Name] : String | semmle.label | access to property Model : TestModel [property Name] : String |
+| TestPage.cshtml:5:16:5:25 | access to property Name | semmle.label | access to property Name |
+subpaths
+#select
+| TestPage.cshtml:5:16:5:25 | access to property Name | TestModel.cs:13:21:13:28 | call to method source : String | TestPage.cshtml:5:16:5:25 | access to property Name | Xss |
+| TestPage.cshtml:5:16:5:25 | access to property Name | TestModel.cs:18:16:18:23 | call to method source : String | TestPage.cshtml:5:16:5:25 | access to property Name | Xss |

csharp/ql/test/query-tests/Security Features/CWE-079/XssPageModels/test.ql

@@ -0,0 +1,25 @@
+/**
+ * @kind path-problem
+ */
+
+import csharp
+import semmle.code.csharp.security.dataflow.XSSQuery
+import semmle.code.csharp.security.dataflow.XSSSinks
+
+module TestXssTrackingConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source.asExpr().(MethodCall).getTarget().getName() = "source"
+  }
+
+  predicate isSink = XssTrackingConfig::isSink/1;
+
+  predicate isBarrier = XssTrackingConfig::isBarrier/1;
+}
+
+module TestXss = TaintTracking::Global<TestXssTrackingConfig>;
+
+import TestXss::PathGraph
+
+from TestXss::PathNode source, TestXss::PathNode sink
+where TestXss::flowPath(source, sink)
+select sink, source, sink, "Xss"