CWE-863: Incorrect Authorization - C

Overview

In ASP.NET Core, Incorrect Authorization usually shows up as an [Authorize(Roles = "...")] attribute or a hand-written if check that validates the wrong thing: a denylist role comparison that refuses a known-bad list and admits every other role, failing open on a value the developer never anticipated, or a check that confirms a resource exists without confirming the current user actually owns it. It also appears as resource-based authorization implemented for one action but not a sibling one, or a policy handler that forgets to deny explicitly on an unmatched branch. The fix is to replace ad hoc role comparisons with policy-based authorization and add resource-based authorization (IAuthorizationHandler) so ownership is checked alongside resource type, not instead of it.

Common Vulnerable Patterns

Denylist Role Comparison

// VULNERABLE - denylist fails open on any role value not explicitly excluded
private static readonly HashSet<string> BlockedRoles = new() { "Guest", "Viewer" };

[HttpDelete("{id}")]
public IActionResult DeleteOrder(int id)
{
    if (BlockedRoles.Contains(User.FindFirstValue(ClaimTypes.Role)))
    {
        return Forbid();
    }

    // Every other role reaches this line: "Support" (added after this check
    // was written), "guest" (case mismatch against the blocked value), and a
    // null claim from an account that never had one set.
    _orderRepository.Delete(id);
    return NoContent();
}

// Attack: authenticate with a role of "Support" (a value the list never anticipated)
// Result: "Support" is not in BlockedRoles, so the delete proceeds

Why this is vulnerable: A denylist names who is refused and admits everyone else, so it is wrong by default for every value it was never told about - and roles are added by people who are not reading this controller. HashSet<string>.Contains is an ordinal match, so "guest" is a different role from "Guest" as far as this line is concerned, and a missing claim yields null, which is in no list and is therefore allowed. An allowlist refuses all three without having to anticipate any of them.

Resource-Type Check Without Ownership

// VULNERABLE - confirms the order exists, never that the caller owns it
[HttpPut("{id}")]
[Authorize]
public IActionResult UpdateOrder(int id, [FromBody] OrderDto dto)
{
    var order = _orderRepository.Find(id);
    if (order is null)
    {
        return NotFound();
    }

    // Any authenticated user can update any order - the resource type
    // (an Order exists) is checked, but ownership never is.
    order.Status = dto.Status;
    _orderRepository.Save(order);
    return NoContent();
}

// Attack: an authenticated low-privilege user requests PUT /orders/{someoneElsesId}
// Result: the update succeeds because [Authorize] only proves authentication,
// and no code compares order.OwnerId to the caller

Why this is vulnerable: [Authorize] with no arguments asserts that someone is signed in and nothing else, so every authenticated account is equivalent as far as this action is concerned. The id in the route is the only thing selecting a record, and it is supplied by the caller.

The NotFound() branch is the part that survives a partial fix. Returning 404 for a missing order and 204 for one that exists tells the caller which identifiers are real even before the update lands - so an ownership check added later must return the same response for "does not exist" and "not yours", or the endpoint remains an enumeration oracle for the whole table.

Check Missing on a Duplicate Path

// VULNERABLE - the ownership check exists on GetOrder but not on the
// bulk endpoint that reaches the same data
[HttpGet("{id}")]
public async Task<IActionResult> GetOrder(int id, [FromServices] IAuthorizationService authService)
{
    var order = await _orderRepository.FindAsync(id);
    var result = await authService.AuthorizeAsync(User, order, "OwnsOrder");
    if (!result.Succeeded) return Forbid();
    return Ok(order);
}

[HttpPost("bulk-export")]
public async Task<IActionResult> BulkExport([FromBody] int[] orderIds)
{
    // No authorization check here - added later, never wired to OwnsOrder
    var orders = await _orderRepository.FindManyAsync(orderIds);
    return Ok(orders);
}

Why this is vulnerable: The guarded and unguarded actions reach the same rows, so the effective authorization on the data is whichever route is weakest - and nothing in the type system, the tests or a review of the protected action reveals that a second door exists.

This is the shape that most often survives remediation, because the finding names one route and the fix is applied there. Before closing one of these, search for every action touching the same repository or entity rather than every action matching the reported URL: the duplicate is usually a bulk endpoint, an admin variant, or an older versioned route left in place for a client nobody has migrated.

Secure Patterns

Resource-Based Authorization with Ownership

// SECURE - resource-based authorization checks ownership, not just resource type
// Register once: services.AddScoped<IAuthorizationHandler, OrderOwnerHandler>();
public class OrderOwnerRequirement : IAuthorizationRequirement { }

public class OrderOwnerHandler : AuthorizationHandler<OrderOwnerRequirement, Order>
{
    protected override Task HandleRequirementAsync(
        AuthorizationHandlerContext context,
        OrderOwnerRequirement requirement,
        Order resource)
    {
        var userId = context.User.FindFirstValue(ClaimTypes.NameIdentifier);

        // Ownership is compared against a server-loaded resource field,
        // never against a client-supplied ownerId.
        if (userId is not null && resource.OwnerId == userId)
        {
            context.Succeed(requirement);
        }
        // No explicit Succeed() call means the requirement fails by default -
        // there is no path here that allows access without matching ownership.
        return Task.CompletedTask;
    }
}

[ApiController]
[Route("api/orders")]
public class OrdersController : ControllerBase
{
    private readonly IAuthorizationService _authorizationService;
    private readonly IOrderRepository _orders;

    public OrdersController(IAuthorizationService authorizationService, IOrderRepository orders)
    {
        _authorizationService = authorizationService;
        _orders = orders;
    }

    [HttpPut("{id}")]
    public async Task<IActionResult> UpdateOrder(int id, [FromBody] OrderDto dto)
    {
        var order = await _orders.FindAsync(id);

        // Not found and not yours produce the same 404. Returning Forbid()
        // for a row that exists confirms the ID is real, which is exactly
        // the enumeration oracle the vulnerable version above leaves behind.
        if (order is null)
        {
            return NotFound();
        }

        var authResult = await _authorizationService.AuthorizeAsync(
            User, order, new OrderOwnerRequirement());
        if (!authResult.Succeeded)
        {
            return NotFound();
        }

        order.Status = dto.Status;
        await _orders.SaveAsync(order);
        return NoContent();
    }

    [HttpPost("bulk-export")]
    public async Task<IActionResult> BulkExport([FromBody] int[] orderIds)
    {
        var orders = await _orders.FindManyAsync(orderIds);

        // The same handler is invoked for every entry point that reaches
        // Order data, including endpoints added after the original fix.
        var authorized = new List<Order>();
        foreach (var order in orders)
        {
            var result = await _authorizationService.AuthorizeAsync(User, order, new OrderOwnerRequirement());
            if (result.Succeeded)
            {
                authorized.Add(order);
            }
        }

        return Ok(authorized);
    }
}

Why this works: the handler loads the resource server-side and compares its OwnerId field against the NameIdentifier claim on the authenticated ClaimsPrincipal, so a valid session alone is never sufficient - the caller must actually own the specific Order instance. Because there is no explicit Succeed() call for any branch other than a matching owner, an unmatched or error condition denies by default instead of falling through to an implicit allow. Calling AuthorizeAsync from both the single-resource action and the bulk endpoint means a fix applied to one path cannot be silently skipped by a duplicate one.

The failed authorization returns NotFound(), not Forbid(), and that is the point of the split responses. Forbid() is the honest status for "you may not do this", but on a resource selected by a caller-supplied ID it also answers a question the caller was not entitled to ask: whether the ID exists. A 403 on /api/orders/4187 and a 404 on /api/orders/4188 together enumerate the table one request at a time, which is the finding the vulnerable pattern above warns about - and it survives adding the ownership check, because adding the check is what introduces the second status code. Use Forbid() where the decision does not depend on a resource the caller named: a role that may not reach the action at all, or an endpoint whose existence is not a secret.

A more durable form is to make the two indistinguishable structurally, by scoping the lookup rather than remembering to match the status codes: _orders.FindForOwnerAsync(id, ownerId) returns null for a row that does not exist and for one owned by someone else, so there is only one branch to get right. That is the same move as filtering the queryset in Django or adding an ownerId clause to the Mongo filter - it makes ownership part of the question rather than a check applied to the answer. Keep the AuthorizeAsync call alongside it: the scoped query cannot express a role rule, and the requirement handler is where an admin exception belongs.

Allowlist Role Check Combined with Ownership

// SECURE - explicit allowlist, and admins bypass ownership by design (not by omission)
private static readonly HashSet<string> AllowedRoles = new() { "Admin", "Editor" };

[HttpDelete("{id}")]
[Authorize]
public async Task<IActionResult> DeleteOrder(int id)
{
    var role = User.FindFirstValue(ClaimTypes.Role);
    if (role is null || !AllowedRoles.Contains(role))
    {
        return Forbid();
    }

    var order = await _orders.FindAsync(id);

    // Both branches answer 404 - see UpdateOrder above
    if (order is null)
    {
        return NotFound();
    }

    if (role != "Admin" && order.OwnerId != User.FindFirstValue(ClaimTypes.NameIdentifier))
    {
        return NotFound();
    }

    await _orders.DeleteAsync(id);
    return NoContent();
}

Why this works: the role comparison is now an explicit allowlist (AllowedRoles.Contains), so a role value the developer never anticipated - or a null claim - is denied instead of silently passing. The ownership check runs for every non-admin role rather than being skipped, and the admin bypass is a deliberate, visible condition rather than the accidental consequence of a list that admitted everything it had not been told to refuse.

Note which denial keeps Forbid() and which became NotFound(). The role check answers Forbid() because it is decided before any order is loaded, so it reveals nothing about which IDs exist; the ownership check answers NotFound() for the reason given above. The distinction is what the response is allowed to leak, not how severe the denial is.

Testing

Role boundary: call the endpoint with a role value the allowlist does not recognize (a typo, a new role, a null claim) and confirm the response is 403 Forbidden, not a successful action.
Cross-owner access: authenticate as one user and request another user's resource by ID through every action that touches it - GET, PUT, PATCH, DELETE, and any bulk endpoint - confirming each one is denied independently.
Boolean logic regression: write a unit test directly against the AuthorizationHandler or policy expression for the exact combination of role and ownership that a prior inversion or short-circuit bug would have allowed.
Normal access: confirm an authenticated owner (and an admin, where intended) can still complete the action successfully - a fix that over-corrects and denies legitimate access is also a defect.
Use WebApplicationFactory<TStartup> integration tests to exercise the real middleware pipeline rather than only unit-testing the handler in isolation, since fallback policy and middleware ordering bugs will not surface in a handler-only test.

Common Pitfalls

Fixing only the flagged action: Correcting the ownership check on GetOrder while BulkExport, an admin-only variant, or a GraphQL resolver for the same entity retains the original gap, because each was wired to authorization independently instead of through one shared handler.
Treating [Authorize] as sufficient for object-level access: [Authorize] alone confirms the caller is authenticated; it says nothing about whether they own the specific resource id in the route, so resource-returning or resource-mutating actions still need an explicit AuthorizeAsync call.
Trusting a role or owner ID from the request body: Reading dto.OwnerId or dto.Role instead of User.FindFirstValue(...) lets an attacker set the value the check compares against.
Missing Succeed() treated as ambiguous rather than denied: Some custom authorization code paths return true/Allow by default when no branch explicitly matches; AuthorizationHandler avoids this by design (no Succeed() call means failure), but hand-written if logic outside the framework does not get this guarantee - it must be written that way.