diff --git a/csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll b/csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll index abdd81646828..37b0ff2884f9 100644 --- a/csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll +++ b/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) { diff --git a/csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql b/csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql index 77a3f2b59450..8a568c6eca57 100644 --- a/csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql +++ b/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." diff --git a/csharp/ql/src/change-notes/2026-08-27-csrf-autovalidate.md b/csharp/ql/src/change-notes/2026-08-27-csrf-autovalidate.md new file mode 100644 index 000000000000..7ba5dbe4467c --- /dev/null +++ b/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. diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/MissingAntiForgeryTokenValidation.cs b/csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/MissingAntiForgeryTokenValidation.cs new file mode 100644 index 000000000000..438ad03f3200 --- /dev/null +++ b/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()); + }); + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/MissingAntiForgeryTokenValidation.expected b/csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/MissingAntiForgeryTokenValidation.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/MissingAntiForgeryTokenValidation.qlref b/csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/MissingAntiForgeryTokenValidation.qlref new file mode 100644 index 000000000000..5e1ab2426c65 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/MissingAntiForgeryTokenValidation.qlref @@ -0,0 +1 @@ +query: Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/options b/csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/options new file mode 100644 index 000000000000..698ad488b6d4 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-352/global-aspnetcore/options @@ -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