Review state-changing endpoints opened to anonymous callers

Rule ND3130 Security

Matches actions carrying both [AllowAnonymous] and a state-changing HTTP verb attribute. [AllowAnonymous] wins over every [Authorize] on the class, the base class and the global filters, so such an action answers any unauthenticated request, whatever the rest of the configuration says. That is intended for sign-in, sign-up, password-recovery, webhook and callback endpoints, which the rule skips by name, and rarely intended anywhere else.

This is CWE-284 Improper Access Control (OWASP Top 10 A01 Broken Access Control).

Flagged Code
[Authorize]
public class CommentsController : ControllerBase
{
   [AllowAnonymous]                                     // flagged: wins over [Authorize]
   [HttpDelete("{id}")]
   public IActionResult DeleteComment(int id) { _comments.Remove(id); return NoContent(); }
}
Fixed Code
[Authorize]
public class CommentsController : ControllerBase
{
   [HttpDelete("{id}")]                                 // the class policy applies again
   public IActionResult DeleteComment(int id) { _comments.Remove(id); return NoContent(); }

   [AllowAnonymous]
   [HttpPost("login")]                                  // sign-in endpoints are skipped by name
   public IActionResult Login(LoginModel model) => SignInAsync(model);
}

How to fix

Remove [AllowAnonymous] so the controller or the global policy applies again. When the endpoint genuinely must be reachable without a session, protect it another way (a signed payload, a shared secret header, a rate limit) and suppress the issue with that justification.

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 endpoints opened to anonymous callers</Name>
// <Id>ND3130:ReviewStateChangingEndpointsOpenedToAnonymousCallers</Id>

warnif count > 0

// Third-party types are part of the code model only when the application uses them.
let anonymousAttrs = ThirdParty.Types.WithFullNameIn(
  "Microsoft.AspNetCore.Authorization.AllowAnonymousAttribute",
  "System.Web.Mvc.AllowAnonymousAttribute",
  "System.Web.Http.AllowAnonymousAttribute").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()

where anonymousAttrs.Any() && stateChangingVerbAttrs.Any()

from m in Application.Methods.TaggedWithAnyAttributes(anonymousAttrs)
where m.AttributeClassesThatTagMe.Any(a => stateChangingVerbAttrs.Contains(a)) &&

      // Sign-in, sign-up and password-recovery endpoints have to be anonymous.
     !m.SimpleNameContainsAny(
        "Login", "SignIn", "SignUp", "Authenticate", "Register", "Token",
        "ForgotPassword", "ResetPassword", "QuickConnect", "Webhook", "Callback")

select new {
  m,
  m.ParentType,
  Debt = 15.ToMinutes().ToDebt(),
  Severity = Severity.High
}

//<Expl>
// The action *{0}* of the *{1}* changes state (POST, PUT, DELETE or PATCH)
// and is explicitly opened to anonymous callers by *[AllowAnonymous]*,
// although its name does not suggest a sign-in or password-recovery endpoint.
//</Expl>

// <Desc>
// Matches actions carrying both *[AllowAnonymous]* and a state-changing
// HTTP verb attribute. *[AllowAnonymous]* wins over every *[Authorize]* on
// the class, the base class and the global filters, so such an action
// answers any unauthenticated request, whatever the rest of the
// configuration says. That is intended for sign-in, sign-up,
// password-recovery, webhook and callback endpoints, which the rule skips
// by name, and rarely intended anywhere else.
//
// This is CWE-284 *Improper Access Control* (OWASP Top 10 A01 Broken
// Access Control).
//
// **Flagged Code**
// ```csharp
// [Authorize]
// public class CommentsController : ControllerBase
// {
//    [AllowAnonymous]                                     // flagged: wins over [Authorize]
//    [HttpDelete("{id}")]
//    public IActionResult DeleteComment(int id) { _comments.Remove(id); return NoContent(); }
// }
// ```
//
// **Fixed Code**
// ```csharp
// [Authorize]
// public class CommentsController : ControllerBase
// {
//    [HttpDelete("{id}")]                                 // the class policy applies again
//    public IActionResult DeleteComment(int id) { _comments.Remove(id); return NoContent(); }
//
//    [AllowAnonymous]
//    [HttpPost("login")]                                  // sign-in endpoints are skipped by name
//    public IActionResult Login(LoginModel model) => SignInAsync(model);
// }
// ```
// </Desc>

// <HowToFix>
// Remove *[AllowAnonymous]* so the controller or the global policy applies
// again. When the endpoint genuinely must be reachable without a session,
// protect it another way (a signed payload, a shared secret header, a rate
// limit) and suppress the issue with that justification.
// </HowToFix>

Open in the interactive Rules Explorer · All default rules

Security: all 39 items