Merge pull request #10050 from michaelnebel/csharp/asproutingendpoints

C#: ASP.NET MapGet Routing endpoints (Remote Flow Sources)

Author: michaelnebel

csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll

@@ -357,3 +357,36 @@ class MicrosoftAspNetCoreHttpHtmlString extends Class {
     this.hasQualifiedName("Microsoft.AspNetCore.Html", "HtmlString")
   }
 }
+
+/**
+ * The `Microsoft.AspNetCore.Builder.EndpointRouteBuilderExtensions` class.
+ */
+class MicrosoftAspNetCoreBuilderEndpointRouteBuilderExtensions extends Class {
+  MicrosoftAspNetCoreBuilderEndpointRouteBuilderExtensions() {
+    this.hasQualifiedName("Microsoft.AspNetCore.Builder", "EndpointRouteBuilderExtensions")
+  }
+
+  /** Gets the `Map` extension method. */
+  Method getMapMethod() { result = this.getAMethod("Map") }
+
+  /** Gets the `MapGet` extension method. */
+  Method getMapGetMethod() { result = this.getAMethod("MapGet") }
+
+  /** Gets the `MapPost` extension method. */
+  Method getMapPostMethod() { result = this.getAMethod("MapPost") }
+
+  /** Gets the `MapPut` extension method. */
+  Method getMapPutMethod() { result = this.getAMethod("MapPut") }
+
+  /** Gets the `MapDelete` extension method. */
+  Method getMapDeleteMethod() { result = this.getAMethod("MapDelete") }
+
+  /** Get a `Map` like extenion methods. */
+  Method getAMapMethod() {
+    result =
+      [
+        this.getMapMethod(), this.getMapGetMethod(), this.getMapPostMethod(),
+        this.getMapPutMethod(), this.getMapDeleteMethod()
+      ]
+  }
+}

csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll

@@ -171,6 +171,35 @@ class ActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode {
 /** A data flow source of remote user input (ASP.NET Core). */
 abstract class AspNetCoreRemoteFlowSource extends RemoteFlowSource { }
 
+private predicate reachesMapGetArg(DataFlow::Node n) {
+  exists(MethodCall mc |
+    mc.getTarget() = any(MicrosoftAspNetCoreBuilderEndpointRouteBuilderExtensions c).getAMapMethod() and
+    n.asExpr() = mc.getArgument(2)
+  )
+  or
+  exists(DataFlow::Node mid | reachesMapGetArg(mid) |
+    DataFlow::localFlowStep(n, mid) or
+    n.asExpr() = mid.asExpr().(DelegateCreation).getArgument()
+  )
+}
+
+/** A parameter to a routing method delegate. */
+class AspNetCoreRoutingMethodParameter extends AspNetCoreRemoteFlowSource, DataFlow::ParameterNode {
+  AspNetCoreRoutingMethodParameter() {
+    exists(DataFlow::Node n, Callable c |
+      reachesMapGetArg(n) and
+      c.getAParameter() = this.asParameter() and
+      c.isSourceDeclaration()
+    |
+      n.asExpr() = c
+      or
+      n.asExpr().(CallableAccess).getTarget().getUnboundDeclaration() = c
+    )
+  }
+
+  override string getSourceType() { result = "ASP.NET Core routing endpoint." }
+}
+
 /**
  * Data flow for ASP.NET Core.
  *

csharp/ql/src/change-notes/2022-08-16-aspnetcore-remoteflowsources.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Parameters of delegates passed to routing endpoint calls like `MapGet` in ASP.NET Core are now considered remote flow sources.

csharp/ql/test/library-tests/dataflow/flowsources/aspremote/AspRemoteFlowSource.cs

@@ -1,4 +1,6 @@
+using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Mvc;
+using System;
 
 namespace Testing
 {
@@ -20,4 +22,40 @@ public object MyAction(ViewModel viewModel)
             throw null;
         }
     }
+
+    public class Item
+    {
+        public string Tainted { get; set; }
+    }
+
+    public class AspRoutingEndpoints
+    {
+        public delegate void MapGetHandler(string param);
+
+        public void HandlerMethod(string param) { }
+
+        public void M1(string[] args)
+        {
+            var builder = WebApplication.CreateBuilder(args);
+            var app = builder.Build();
+
+            // The delegate parameters are considered flow sources.
+            app.MapGet("/api/redirect/{newUrl}", (string newUrl) => { });
+            app.MapGet("/{myApi}/redirect/{myUrl}", (string myApi, string myUrl) => { });
+
+            Action<string> handler = (string lambdaParam) => { };
+            app.MapGet("/api/redirect/{lambdaParam}", handler);
+
+            MapGetHandler handler2 = HandlerMethod;
+            app.MapGet("/api/redirect/{mapGetParam}", handler2);
+
+            app.MapPost("/api/redirect/{mapPostParam}", (string mapPostParam) => { });
+            app.MapPut("/api/redirect/{mapPutParam}", (string mapPutParam) => { });
+            app.MapDelete("/api/redirect/{mapDeleteParam}", (string mapDeleteParam) => { });
+
+            app.MapPost("/items", (Item item) => { });
+
+            app.Run();
+        }
+    }
 }

csharp/ql/test/library-tests/dataflow/flowsources/aspremote/aspRemoteFlowSource.expected

@@ -1,4 +1,14 @@
 remoteFlowSourceMembers
-| AspRemoteFlowSource.cs:8:23:8:31 | RequestId |
+| AspRemoteFlowSource.cs:10:23:10:31 | RequestId |
+| AspRemoteFlowSource.cs:28:23:28:29 | Tainted |
 remoteFlowSources
-| AspRemoteFlowSource.cs:18:42:18:50 | viewModel |
+| AspRemoteFlowSource.cs:20:42:20:50 | viewModel |
+| AspRemoteFlowSource.cs:35:42:35:46 | param |
+| AspRemoteFlowSource.cs:43:58:43:63 | newUrl |
+| AspRemoteFlowSource.cs:44:61:44:65 | myApi |
+| AspRemoteFlowSource.cs:44:75:44:79 | myUrl |
+| AspRemoteFlowSource.cs:46:46:46:56 | lambdaParam |
+| AspRemoteFlowSource.cs:52:65:52:76 | mapPostParam |
+| AspRemoteFlowSource.cs:53:63:53:73 | mapPutParam |
+| AspRemoteFlowSource.cs:54:69:54:82 | mapDeleteParam |
+| AspRemoteFlowSource.cs:56:41:56:44 | item |