Review state-changing MVC actions without anti-forgery validation

Rule ND3119 Security

Matches view-based MVC controllers (deriving from Microsoft.AspNetCore.Mvc.Controller or System.Web.Mvc.Controller) whose POST/PUT/DELETE/PATCH actions are not covered by [ValidateAntiForgeryToken] or [AutoValidateAntiforgeryToken] on the action, the class chain, or a global filter, and are not explicitly exempted with [IgnoreAntiforgeryToken]. Controllers or actions tagged [ApiController] or [Consumes(...)] are skipped: they are not reachable from an HTML form. Cookie-authenticated form posts without a token are the textbook CSRF (CWE-352).

Flagged Code
public class AccountController : Controller
{
   [HttpPost]                                           // flagged: no anti-forgery validation
   public IActionResult UpdateEmail(string email)
   {
      _accounts.SetEmail(User.Identity.Name, email);
      return RedirectToAction("Index");
   }
}
Fixed Code
public class AccountController : Controller
{
   [HttpPost]
   [ValidateAntiForgeryToken]                           // the form must carry the token
   public IActionResult UpdateEmail(string email)
   {
      _accounts.SetEmail(User.Identity.Name, email);
      return RedirectToAction("Index");
   }
}

How to fix

Register AutoValidateAntiforgeryTokenAttribute as a global filter in AddControllersWithViews(o => o.Filters.Add(new AutoValidateAntiforgeryTokenAttribute())) and emit the token in forms with the asp-antiforgery tag helper (on by default for <form>), or tag each unsafe action with [ValidateAntiForgeryToken]. Exempt token-authenticated endpoints explicitly with [IgnoreAntiforgeryToken].

CQLinq source

This rule is written in CQLinq, C# LINQ over the NDepend code model. It ships with every new NDepend project: you can read it, adapt it, or use it as a starting point for your own custom rules.

// <Name>Review state-changing MVC actions without anti-forgery validation</Name>
// <Id>ND3119:ReviewStateChangingMvcActionsWithoutAntiForgeryValidation</Id>
warnif count > 0

// Only view-based MVC controllers are concerned: API controllers authenticate with tokens, not cookies.
// Third-party types are part of the code model only when the application uses them.
let mvcControllerTypes = ThirdParty.Types.WithFullNameIn("Microsoft.AspNetCore.Mvc.Controller", "System.Web.Mvc.Controller").ToArray()
where mvcControllerTypes.Any()

let validateAttrs = ThirdParty.Types.WithFullNameIn(
  "Microsoft.AspNetCore.Mvc.ValidateAntiForgeryTokenAttribute",
  "Microsoft.AspNetCore.Mvc.AutoValidateAntiforgeryTokenAttribute",
  "System.Web.Mvc.ValidateAntiForgeryTokenAttribute").ToHashSetEx()
let ignoreAttrs = ThirdParty.Types.WithFullName("Microsoft.AspNetCore.Mvc.IgnoreAntiforgeryTokenAttribute").ToHashSetEx()
let apiControllerAttrs = ThirdParty.Types.WithFullName("Microsoft.AspNetCore.Mvc.ApiControllerAttribute").ToHashSetEx()
let consumesAttrs = ThirdParty.Types.WithFullName("Microsoft.AspNetCore.Mvc.ConsumesAttribute").ToHashSetEx()
let stateChangingVerbAttrs = ThirdParty.Types.WithFullNameIn(
  "Microsoft.AspNetCore.Mvc.HttpPostAttribute", "Microsoft.AspNetCore.Mvc.HttpPutAttribute",
  "Microsoft.AspNetCore.Mvc.HttpDeleteAttribute", "Microsoft.AspNetCore.Mvc.HttpPatchAttribute",
  "System.Web.Mvc.HttpPostAttribute", "System.Web.Mvc.HttpPutAttribute", "System.Web.Mvc.HttpDeleteAttribute").ToHashSetEx()

// A global filter validates every unsafe request: nothing to report.
let appHasGlobalValidation = Application.Methods.Any(m => validateAttrs.Any(a => m.IsUsing(a)))
where !appHasGlobalValidation

let tagged = new Func<IAttributeTarget, HashSet<IType>, bool>((x, attrs) => x.AttributeClassesThatTagMe.Any(a => attrs.Contains(a)))

from t in Application.Types.ThatDeriveFromAny(mvcControllerTypes)
where !t.IsAbstract && !t.IsGeneratedByCompiler
let classChain = t.BaseClasses.Concat(new[] { t }).ToArray()
where !classChain.Any(c => tagged(c, validateAttrs) || tagged(c, ignoreAttrs) || tagged(c, apiControllerAttrs) || tagged(c, consumesAttrs))

let unvalidatedActions = t.Methods.Where(m =>
      m.IsPublic && !m.IsStatic && !m.IsGeneratedByCompiler &&
      tagged(m, stateChangingVerbAttrs) &&
     !tagged(m, validateAttrs) && !tagged(m, ignoreAttrs) &&
     !tagged(m, consumesAttrs)).ToArray()      // [Consumes("application/json")]: not reachable from an HTML form
where unvalidatedActions.Length > 0

select new {
  t,
  unvalidatedActions,
  Debt = (10 * unvalidatedActions.Length).ToMinutes().ToDebt(),
  Severity = Severity.High
}

//<Expl>
// The MVC controller *{0}* exposes state-changing action(s) *{1}* (POST, PUT,
// DELETE or PATCH) without *[ValidateAntiForgeryToken]*: a malicious page can
// submit a form to them with the victim's cookies (Cross-Site Request Forgery).
//</Expl>

//<Desc>
// Matches view-based MVC controllers (deriving from
// *Microsoft.AspNetCore.Mvc.Controller* or *System.Web.Mvc.Controller*)
// whose POST/PUT/DELETE/PATCH actions are not covered by
// *[ValidateAntiForgeryToken]* or *[AutoValidateAntiforgeryToken]* on the
// action, the class chain, or a global filter, and are not explicitly
// exempted with *[IgnoreAntiforgeryToken]*. Controllers or actions tagged
// *[ApiController]* or *[Consumes(...)]* are skipped: they are not
// reachable from an HTML form. Cookie-authenticated form posts without a
// token are the textbook CSRF (CWE-352).
//
// **Flagged Code**
// ```csharp
// public class AccountController : Controller
// {
//    [HttpPost]                                           // flagged: no anti-forgery validation
//    public IActionResult UpdateEmail(string email)
//    {
//       _accounts.SetEmail(User.Identity.Name, email);
//       return RedirectToAction("Index");
//    }
// }
// ```
//
// **Fixed Code**
// ```csharp
// public class AccountController : Controller
// {
//    [HttpPost]
//    [ValidateAntiForgeryToken]                           // the form must carry the token
//    public IActionResult UpdateEmail(string email)
//    {
//       _accounts.SetEmail(User.Identity.Name, email);
//       return RedirectToAction("Index");
//    }
// }
// ```
//</Desc>

//<HowToFix>
// Register *AutoValidateAntiforgeryTokenAttribute* as a global filter in
// *AddControllersWithViews(o => o.Filters.Add(new AutoValidateAntiforgeryTokenAttribute()))*
// and emit the token in forms with the *asp-antiforgery* tag helper (on by
// default for *<form>*), or tag each unsafe action with
// *[ValidateAntiForgeryToken]*. Exempt token-authenticated endpoints
// explicitly with *[IgnoreAntiforgeryToken]*.
//</HowToFix>

Open in the interactive Rules Explorer · All default rules

Security: all 39 items