diff --git a/csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql b/csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql index 60a022d43d0f..a1cb7f9f506a 100644 --- a/csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql +++ b/csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql @@ -51,6 +51,80 @@ predicate hasGlobalAntiForgeryFilter() { ) } +private class RequireAntiforgeryTokenAttribute extends Attribute { + RequireAntiforgeryTokenAttribute() { + this.getType() + .hasFullyQualifiedName("Microsoft.AspNetCore.Antiforgery", + "RequireAntiforgeryTokenAttribute") + } + + predicate requiresValidation() { + not exists(this.getArgument(0)) + or + this.getArgument(0).isImplicit() + or + this.getArgument(0).getValue() = "true" + } +} + +private predicate hasAspNetCoreAntiForgeryMiddleware() { + exists(MethodCall call | + call.getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Builder", + "AntiforgeryApplicationBuilderExtensions", "UseAntiforgery") + ) +} + +bindingset[method] +private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttributeOnMethod( + Method method +) { + exists(Method attributedMethod | + attributedMethod = method.getOverridee*() and + result = attributedMethod.getAnAttribute() and + not exists(Method closerMethod | + closerMethod = method.getOverridee*() and + closerMethod.getOverridee+() = attributedMethod and + closerMethod.getAnAttribute() instanceof RequireAntiforgeryTokenAttribute + ) + ) +} + +bindingset[controller] +private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttributeOnClass( + Class controller +) { + exists(Class attributedClass | + attributedClass = controller.getBaseClass*() and + result = attributedClass.getAnAttribute() and + not exists(Class closerClass | + closerClass = controller.getBaseClass*() and + closerClass.getBaseClass+() = attributedClass and + closerClass.getAnAttribute() instanceof RequireAntiforgeryTokenAttribute + ) + ) +} + +bindingset[controller, method] +private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttribute( + Class controller, Method method +) { + result = getEffectiveRequireAntiforgeryTokenAttributeOnMethod(method) + or + not exists(getEffectiveRequireAntiforgeryTokenAttributeOnMethod(method)) and + result = getEffectiveRequireAntiforgeryTokenAttributeOnClass(controller) +} + +bindingset[controller, method] +private predicate hasAspNetCoreAntiForgeryValidation(Class controller, Method method) { + method.getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute + or + controller.getABaseType*().getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute + or + hasAspNetCoreAntiForgeryMiddleware() and + getEffectiveRequireAntiforgeryTokenAttribute(controller, method).requiresValidation() +} + predicate isUnvalidatedPostMethod(Class c, Method m) { c.(Controller).getAPostActionMethod() = m and not m.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute and @@ -58,14 +132,16 @@ predicate isUnvalidatedPostMethod(Class c, Method m) { or c.(AspNetCore::MicrosoftAspNetCoreMvcController).getAnActionMethod() = m and m.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute and - not m.getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute and - not c.getABaseType*().getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute + not hasAspNetCoreAntiForgeryValidation(c, m) } Element getAValidatedElement() { any(ValidateAntiForgeryTokenAttribute a).getTarget() = result or any(AspNetCore::ValidateAntiForgeryAttribute a).getTarget() = result + or + hasAspNetCoreAntiForgeryMiddleware() and + any(RequireAntiforgeryTokenAttribute a | a.requiresValidation()).getTarget() = result } from Class c, Method postMethod diff --git a/csharp/ql/src/change-notes/2026-08-09-require-antiforgery-token.md b/csharp/ql/src/change-notes/2026-08-09-require-antiforgery-token.md new file mode 100644 index 000000000000..ce1b6e9418fe --- /dev/null +++ b/csharp/ql/src/change-notes/2026-08-09-require-antiforgery-token.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `cs/web/missing-token-validation` query now recognizes enabled ASP.NET Core `RequireAntiforgeryToken` attributes when antiforgery middleware is used. diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/MissingAntiForgeryTokenValidation.cs b/csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/MissingAntiForgeryTokenValidation.cs new file mode 100644 index 000000000000..32a6e8326555 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/MissingAntiForgeryTokenValidation.cs @@ -0,0 +1,27 @@ +using Microsoft.AspNetCore.Antiforgery; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; + +public class HomeController : Controller +{ + [HttpPost] + [RequireAntiforgeryToken(false)] + public ActionResult DisabledValidation() + { + return View(); + } + + [HttpPost] + public ActionResult MissingValidation() + { + return View(); + } +} + +public class Startup +{ + public void Configure(IApplicationBuilder app) + { + app.UseAntiforgery(); + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/MissingAntiForgeryTokenValidation.expected b/csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/MissingAntiForgeryTokenValidation.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/MissingAntiForgeryTokenValidation.qlref b/csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/MissingAntiForgeryTokenValidation.qlref new file mode 100644 index 000000000000..5e1ab2426c65 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/MissingAntiForgeryTokenValidation.qlref @@ -0,0 +1 @@ +query: Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/options b/csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/options new file mode 100644 index 000000000000..698ad488b6d4 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/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 diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/missing-aspnetcore/MissingAntiForgeryTokenValidation.cs b/csharp/ql/test/query-tests/Security Features/CWE-352/missing-aspnetcore/MissingAntiForgeryTokenValidation.cs index febc12a61255..123ac0f7ec26 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-352/missing-aspnetcore/MissingAntiForgeryTokenValidation.cs +++ b/csharp/ql/test/query-tests/Security Features/CWE-352/missing-aspnetcore/MissingAntiForgeryTokenValidation.cs @@ -1,7 +1,12 @@ using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Antiforgery; +using Microsoft.AspNetCore.Builder; public class HomeController : Controller { + private const bool ValidationEnabled = true; + private const bool ValidationDisabled = false; + // BAD: Anti forgery token has been forgotten [HttpPost] public ActionResult Login() // $ Alert @@ -17,6 +22,46 @@ public ActionResult UpdateDetails() return View(); } + // GOOD: Anti forgery token is required by ASP.NET Core middleware + [HttpPost] + [RequireAntiforgeryToken] + public ActionResult UpdateProfile() + { + return View(); + } + + // GOOD: Explicitly requires anti forgery validation + [HttpPost] + [RequireAntiforgeryToken(true)] + public ActionResult UpdatePreferences() + { + return View(); + } + + // GOOD: Named and constant arguments are supported + [HttpPost] + [RequireAntiforgeryToken(required: ValidationEnabled)] + public ActionResult UpdateSettings() + { + return View(); + } + + // BAD: Explicitly disables anti forgery validation + [HttpPost] + [RequireAntiforgeryToken(false)] + public ActionResult DisabledValidation() // $ Alert + { + return View(); + } + + // BAD: A false constant also disables anti forgery validation + [HttpPost] + [RequireAntiforgeryToken(ValidationDisabled)] + public ActionResult ConstantDisabledValidation() // $ Alert + { + return View(); + } + // No validation required, as this is a GET method. public ActionResult ShowHelp() { @@ -46,6 +91,110 @@ public ActionResult InheritedValidation() } } +// GOOD: Base class requires anti forgery validation +[RequireAntiforgeryToken] +public abstract class AntiforgeryBaseController : Controller +{ +} + +public abstract class IntermediateAntiforgeryController : AntiforgeryBaseController +{ +} + +public class DerivedAntiforgeryController : IntermediateAntiforgeryController +{ + [HttpPost] + public ActionResult InheritedRequiredValidation() + { + return View(); + } +} + +[RequireAntiforgeryToken] +public class ProtectedController : Controller +{ + // GOOD: Controller requires anti forgery validation + [HttpPost] + public ActionResult ProtectedAction() + { + return View(); + } + + // BAD: Action-level metadata overrides the controller metadata + [HttpPost] + [RequireAntiforgeryToken(false)] + public ActionResult DisabledAction() // $ Alert + { + return View(); + } +} + +[RequireAntiforgeryToken(false)] +public class DisabledController : Controller +{ + // BAD: Controller explicitly disables anti forgery validation + [HttpPost] + public ActionResult DisabledControllerAction() // $ Alert + { + return View(); + } + + // GOOD: Action-level metadata overrides the controller metadata + [HttpPost] + [RequireAntiforgeryToken(true)] + public ActionResult EnabledAction() + { + return View(); + } +} + +[RequireAntiforgeryToken] +public abstract class ProtectedBaseController : Controller +{ +} + +[RequireAntiforgeryToken(false)] +public class DisabledDerivedController : ProtectedBaseController +{ + // BAD: Derived controller metadata overrides base controller metadata + [HttpPost] + public ActionResult DisabledInheritedAction() // $ Alert + { + return View(); + } +} + +[AutoValidateAntiforgeryToken] +public class FilterProtectedController : Controller +{ + // GOOD: Disabled middleware metadata does not disable the MVC filter + [HttpPost] + [RequireAntiforgeryToken(false)] + public ActionResult FilterProtectedAction() + { + return View(); + } +} + +public abstract class MethodMetadataBaseController : Controller +{ + [RequireAntiforgeryToken] + public virtual ActionResult InheritedMethodValidation() + { + return View(); + } +} + +public class MethodMetadataController : MethodMetadataBaseController +{ + // GOOD: Method metadata is inherited by the override + [HttpPost] + public override ActionResult InheritedMethodValidation() + { + return View(); + } +} + // BAD: Base class without antiforgery attribute public abstract class UnprotectedBaseController : Controller { @@ -60,3 +209,29 @@ public ActionResult NoInheritedValidation() // $ Alert return View(); } } + +namespace Custom +{ + public class RequireAntiforgeryTokenAttribute : System.Attribute + { + } + + public class CustomAttributeController : Controller + { + // BAD: An unrelated attribute with the same name does not provide validation + [HttpPost] + [RequireAntiforgeryToken] + public ActionResult LookalikeAttribute() // $ Alert + { + return View(); + } + } +} + +public class Startup +{ + public void Configure(IApplicationBuilder app) + { + app.UseAntiforgery(); + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/missing-aspnetcore/MissingAntiForgeryTokenValidation.expected b/csharp/ql/test/query-tests/Security Features/CWE-352/missing-aspnetcore/MissingAntiForgeryTokenValidation.expected index 360b909cd0fa..ceb32e94c984 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-352/missing-aspnetcore/MissingAntiForgeryTokenValidation.expected +++ b/csharp/ql/test/query-tests/Security Features/CWE-352/missing-aspnetcore/MissingAntiForgeryTokenValidation.expected @@ -1,2 +1,8 @@ -| MissingAntiForgeryTokenValidation.cs:7:25:7:29 | Login | Method 'Login' handles a POST request without performing CSRF token validation. | -| MissingAntiForgeryTokenValidation.cs:58:25:58:45 | NoInheritedValidation | Method 'NoInheritedValidation' handles a POST request without performing CSRF token validation. | +| MissingAntiForgeryTokenValidation.cs:12:25:12:29 | Login | Method 'Login' handles a POST request without performing CSRF token validation. | +| MissingAntiForgeryTokenValidation.cs:52:25:52:42 | DisabledValidation | Method 'DisabledValidation' handles a POST request without performing CSRF token validation. | +| MissingAntiForgeryTokenValidation.cs:60:25:60:50 | ConstantDisabledValidation | Method 'ConstantDisabledValidation' handles a POST request without performing CSRF token validation. | +| MissingAntiForgeryTokenValidation.cs:126:25:126:38 | DisabledAction | Method 'DisabledAction' handles a POST request without performing CSRF token validation. | +| MissingAntiForgeryTokenValidation.cs:137:25:137:48 | DisabledControllerAction | Method 'DisabledControllerAction' handles a POST request without performing CSRF token validation. | +| MissingAntiForgeryTokenValidation.cs:161:25:161:47 | DisabledInheritedAction | Method 'DisabledInheritedAction' handles a POST request without performing CSRF token validation. | +| MissingAntiForgeryTokenValidation.cs:207:25:207:45 | NoInheritedValidation | Method 'NoInheritedValidation' handles a POST request without performing CSRF token validation. | +| MissingAntiForgeryTokenValidation.cs:224:29:224:46 | LookalikeAttribute | Method 'LookalikeAttribute' handles a POST request without performing CSRF token validation. | diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/MissingAntiForgeryTokenValidation.cs b/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/MissingAntiForgeryTokenValidation.cs new file mode 100644 index 000000000000..9943c21aff3f --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/MissingAntiForgeryTokenValidation.cs @@ -0,0 +1,19 @@ +using Microsoft.AspNetCore.Antiforgery; +using Microsoft.AspNetCore.Mvc; + +public class HomeController : Controller +{ + [HttpPost] + [ValidateAntiForgeryToken] + public ActionResult FilterValidated() + { + return View(); + } + + [HttpPost] + [RequireAntiforgeryToken] + public ActionResult MetadataWithoutMiddleware() // $ Alert + { + return View(); + } +} diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/MissingAntiForgeryTokenValidation.expected b/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/MissingAntiForgeryTokenValidation.expected new file mode 100644 index 000000000000..2b5360166026 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/MissingAntiForgeryTokenValidation.expected @@ -0,0 +1 @@ +| MissingAntiForgeryTokenValidation.cs:15:25:15:49 | MetadataWithoutMiddleware | Method 'MetadataWithoutMiddleware' handles a POST request without performing CSRF token validation. | diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/MissingAntiForgeryTokenValidation.qlref b/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/MissingAntiForgeryTokenValidation.qlref new file mode 100644 index 000000000000..5e1ab2426c65 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/MissingAntiForgeryTokenValidation.qlref @@ -0,0 +1 @@ +query: Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql diff --git a/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/options b/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/options new file mode 100644 index 000000000000..698ad488b6d4 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-352/require-antiforgery-without-middleware/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