Merge pull request #22431 from michaelnebel/csharp/csrf

C#: The `cs/web/missing-token-validation` query now recognizes an ASP.NET Core `AutoValidateAntiforgeryTokenAttribute`.

Author: michaelnebel

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

@@ -144,6 +144,15 @@ class ValidateAntiForgeryAttribute extends Attribute {
   }
 }
 
+/**
+ * The `Microsoft.AspNetCore.Mvc.AutoValidateAntiforgeryTokenAttribute` class.
+ */
+class AutoValidateAntiforgeryTokenAttribute extends Class {
+  AutoValidateAntiforgeryTokenAttribute() {
+    this.hasFullyQualifiedName("Microsoft.AspNetCore.Mvc", "AutoValidateAntiforgeryTokenAttribute")
+  }
+}
+
 /**
  * A class that has a name like `[Auto...]Validate[...]Anti[Ff]orgery[...Token]` and implements `IFilterMetadata` interface
  * This class can be added to a collection of global `MvcOptions.Filters` collection.
@@ -164,8 +173,8 @@ class MicrosoftAspNetCoreMvcFilterCollection extends Class {
 
   /** Gets an `Add` method. */
   Method getAddMethod() {
-    result = this.getAMethod("Add") or
-    result = this.getABaseType().getAMethod("Add")
+    result = this.getAMethod(["Add", "Add`1"]) or
+    result = this.getABaseType().getAMethod(["Add", "Add`1"])
   }
 }
 
@@ -230,11 +239,20 @@ private Assembly getAnAssemblyFor(Type type) {
   result = getACompilationFor(type).getOutputAssembly()
 }
 
-private predicate isMicrosoftAspNetCoreMvcRegistration(MethodCall call) {
-  call.getTarget()
-      .hasFullyQualifiedName("Microsoft.Extensions.DependencyInjection",
-        ["MvcServiceCollectionExtensions", "MvcCoreServiceCollectionExtensions"],
-        ["AddControllers", "AddControllersWithViews", "AddMvc", "AddMvcCore"])
+/**
+ * A method that is a registration of an ASP.NET Core MVC service, i.e. `AddControllers`, `AddControllersWithViews`, `AddMvc`, or `AddMvcCore`.
+ */
+class MicrosoftAspNetCoreMvcRegistration extends Method {
+  MicrosoftAspNetCoreMvcRegistration() {
+    this.hasFullyQualifiedName("Microsoft.Extensions.DependencyInjection",
+      ["MvcServiceCollectionExtensions", "MvcCoreServiceCollectionExtensions"],
+      ["AddControllers", "AddControllersWithViews", "AddMvc", "AddMvcCore"])
+  }
+}
+
+/** Holds if the method call is a registration of an ASP.NET Core MVC service. */
+predicate isMicrosoftAspNetCoreMvcRegistration(MethodCall call) {
+  call.getTarget() instanceof MicrosoftAspNetCoreMvcRegistration
 }
 
 private predicate isMicrosoftAspNetCoreMvcApplication(Compilation compilation) {

csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql

@@ -12,6 +12,7 @@
  */
 
 import csharp
+import semmle.code.csharp.commons.Compilation
 import semmle.code.csharp.frameworks.system.Web
 import semmle.code.csharp.frameworks.system.web.Helpers
 import semmle.code.csharp.frameworks.system.web.Mvc
@@ -34,20 +35,41 @@ private Method getAStartedMethod() {
   getAStartedMethod().calls(result)
 }
 
-/**
- * Holds if the project has a global anti forgery filter.
- *
- * No AspNetCore case here as the corresponding class doesn't seem to exist.
- */
-predicate hasGlobalAntiForgeryFilter() {
-  // A global filter added
+private predicate hasGlobalWebMvcAntiforgeryFilter(Compilation compilation) {
   exists(MethodCall addGlobalFilter |
     // addGlobalFilter adds a filter to the global filter collection
     addGlobalFilter.getTarget() = any(GlobalFilterCollection gfc).getAddMethod() and
     // The filter is an antiforgery filter
     addGlobalFilter.getArgumentForName("filter").getType() instanceof AntiForgeryAuthorizationFilter and
     // The filter is added by the Application_Start() method
-    getAStartedMethod() = addGlobalFilter.getEnclosingCallable()
+    getAStartedMethod() = addGlobalFilter.getEnclosingCallable() and
+    addGlobalFilter.getFile() = compilation.getAFileCompiled()
+  )
+}
+
+predicate hasGlobalAspNetMvcAntiForgeryFilter(Compilation compilation) {
+  exists(MethodCall addGlobalFilter, MethodCall registrationCall |
+    (
+      // The filter is the `AutoValidateAntiforgeryTokenAttribute` filter.
+      addGlobalFilter.getTarget() =
+        any(AspNetCore::MicrosoftAspNetCoreMvcFilterCollection collection).getAddMethod() and
+      (
+        addGlobalFilter.getArgument(0).getType() instanceof
+          AspNetCore::AutoValidateAntiforgeryTokenAttribute or
+        addGlobalFilter.getArgument(0).(TypeofExpr).getTypeAccess().getTarget() instanceof
+          AspNetCore::AutoValidateAntiforgeryTokenAttribute
+      )
+      or
+      addGlobalFilter.getTarget().getUnboundDeclaration() =
+        any(AspNetCore::MicrosoftAspNetCoreMvcFilterCollection collection).getAddMethod() and
+      addGlobalFilter.getTarget().(ConstructedGeneric).getTypeArgument(0) instanceof
+        AspNetCore::AutoValidateAntiforgeryTokenAttribute
+    ) and
+    // The filter is added in an ASP.NET Core registration call, which is provided as a lambda argument
+    // to the Mvc registration method.
+    registrationCall.getTarget() instanceof AspNetCore::MicrosoftAspNetCoreMvcRegistration and
+    registrationCall.getAnArgument() = addGlobalFilter.getEnclosingCallable() and
+    addGlobalFilter.getFile() = compilation.getAFileCompiled()
   )
 }
 
@@ -67,11 +89,12 @@ private class RequireAntiforgeryTokenAttribute extends Attribute {
   }
 }
 
-private predicate hasAspNetCoreAntiForgeryMiddleware() {
+private predicate hasAspNetCoreAntiForgeryMiddleware(Compilation compilation) {
   exists(MethodCall call |
     call.getTarget()
         .hasFullyQualifiedName("Microsoft.AspNetCore.Builder",
-          "AntiforgeryApplicationBuilderExtensions", "UseAntiforgery")
+          "AntiforgeryApplicationBuilderExtensions", "UseAntiforgery") and
+    call.getFile() = compilation.getAFileCompiled()
   )
 }
 
@@ -106,7 +129,12 @@ private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttr
 class MvcControllerPostMethod extends Method {
   private Controller controller;
 
-  MvcControllerPostMethod() { controller.getAPostActionMethod() = this }
+  MvcControllerPostMethod() {
+    controller.getAPostActionMethod() = this and
+    exists(Compilation compilation | compilation.getAFileCompiled() = this.getFile() |
+      not hasGlobalWebMvcAntiforgeryFilter(compilation)
+    )
+  }
 
   predicate hasValidateAntiForgeryAttribute() {
     this.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute or
@@ -116,10 +144,13 @@ class MvcControllerPostMethod extends Method {
 
 class AspNetCoreControllerPostMethod extends Method {
   private AspNetCore::MicrosoftAspNetCoreMvcController controller;
+  private Compilation compilation;
 
   AspNetCoreControllerPostMethod() {
     controller.getAnActionMethod() = this and
-    this.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute
+    this.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute and
+    compilation.getAFileCompiled() = this.getFile() and
+    not hasGlobalAspNetMvcAntiForgeryFilter(compilation)
   }
 
   predicate hasValidateAntiForgeryAttribute() {
@@ -128,7 +159,7 @@ class AspNetCoreControllerPostMethod extends Method {
   }
 
   predicate hasRequireAntiForgeryAttribute() {
-    hasAspNetCoreAntiForgeryMiddleware() and
+    hasAspNetCoreAntiForgeryMiddleware(compilation) and
     (
       getEffectiveRequireAntiforgeryTokenAttributeOnMethod(this).requiresValidation()
       or
@@ -157,7 +188,7 @@ Element getAValidatedElement() {
   or
   any(AspNetCore::ValidateAntiForgeryAttribute a).getTarget() = result
   or
-  hasAspNetCoreAntiForgeryMiddleware() and
+  hasAspNetCoreAntiForgeryMiddleware(_) and
   any(RequireAntiforgeryTokenAttribute a | a.requiresValidation()).getTarget() = result
 }
 
@@ -167,9 +198,7 @@ where
   // Verify that validate anti forgery token attributes are used somewhere within this project, to
   // avoid reporting false positives on projects that use an alternative approach to mitigate CSRF
   // issues.
-  exists(getAValidatedElement()) and
-  // Also ignore cases where a global anti forgery filter is in use.
-  not hasGlobalAntiForgeryFilter()
+  exists(getAValidatedElement())
 select postMethod,
   "Method '" + postMethod.getName() +
     "' handles a POST request without performing CSRF token validation."

csharp/ql/src/change-notes/2026-08-27-csrf-autovalidate.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `cs/web/missing-token-validation` query now recognizes an ASP.NET Core `AutoValidateAntiforgeryTokenAttribute` registered as a global MVC filter through `AddControllersWithViews` (and friends), avoiding false-positive results for covered actions.

csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/MissingAntiForgeryTokenValidation.cs

@@ -0,0 +1,39 @@
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc.ViewFeatures;
+using Microsoft.AspNetCore.Routing;
+using Microsoft.Extensions.DependencyInjection;
+
+public class HomeController : Controller
+{
+    // GOOD: This is validated by the global filter.
+    [HttpPost]
+    public ActionResult Login()
+    {
+        return View();
+    }
+
+    // GOOD: Antiforgery token is validated explicitly.
+    [HttpPost]
+    [ValidateAntiForgeryToken]
+    public ActionResult UpdateDetails()
+    {
+        return View();
+    }
+}
+
+public class Program
+{
+    public static void Main(string[] args)
+    {
+        var builder = WebApplication.CreateBuilder(args);
+
+        // Register MVC controllers and Razor views.
+        // The global filter automatically validates antiforgery tokens
+        // for unsafe HTTP methods such as POST, PUT, PATCH, and DELETE.
+        builder.Services.AddControllersWithViews(options =>
+        {
+            options.Filters.Add(new AutoValidateAntiforgeryTokenAttribute());
+        });
+    }
+}

csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/MissingAntiForgeryTokenValidation.expected

@@ -0,0 +1 @@
+query: Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql

csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/MissingAntiForgeryTokenValidation.qlref

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