CWE-863: Incorrect Authorization - PHP
Overview
In Laravel and similar PHP frameworks, Incorrect Authorization commonly appears as a Policy or controller check that compares role against a denylist, failing open on a role value the check never anticipated, or a Policy method that checks the resource's class or existence but never compares its owning user ID to the authenticated user. It also appears as a check present in one controller action but missing from a sibling action for the same model, or a Policy method that omits a return on an unmatched branch, which yields null - denied by authorize() and can(), but read as an allow by code that tests the result itself with !== false. Fix by writing Policy methods that combine an explicit role allowlist with an ownership comparison, and by calling $this->authorize() on every action that touches the resource.
Common Vulnerable Patterns
Denylist Role Comparison
// VULNERABLE - denylist fails open on any role value not explicitly excluded
class OrderController extends Controller
{
private const BLOCKED_ROLES = ['guest', 'viewer'];
public function destroy(Order $order)
{
if (in_array(auth()->user()->role, self::BLOCKED_ROLES, true)) {
abort(403);
}
// Every other role reaches this line: 'support' (introduced later),
// 'Guest' (case mismatch against the blocked value), and a null role
// from an incomplete migration.
$order->delete();
return response()->noContent();
}
}
// Attack: authenticate with role = 'support' (a value the list never anticipated)
// Result: 'support' is not in BLOCKED_ROLES, so the delete proceeds
Why this is vulnerable: A denylist enumerates who is refused and admits everyone else, so it fails open for every role that did not exist when it was written - and roles are added by people who are not reading this controller.
PHP's comparison semantics add two more ways through. A loose != applies type juggling, so values that are not the string being blocked can still compare equal or unequal in ways the author did not intend, and a user whose role column is null compares unequal to the blocked value and is therefore permitted. An allowlist refuses all three cases without being told about them.
Policy Checking Resource Class, Not Instance
// VULNERABLE - checks that the user can act on Orders in general, never
// that this particular order belongs to them
class OrderController extends Controller
{
public function update(Request $request, Order $order)
{
// Class-level check: confirms the user has *some* update permission
// on the Order model, not that they own $order specifically.
if (!$request->user()->can('update', Order::class)) {
abort(403);
}
$order->update($request->validate(['status' => 'required|string']));
return response()->json($order);
}
}
// Attack: an authenticated low-privilege user requests PUT /orders/{someoneElsesId}
// Result: can('update', Order::class) passes because it only evaluates
// against the model class, never the loaded $order instance
Why this is vulnerable: can('update', Order::class) and can('update', $order) look interchangeable and are not. Laravel resolves the class form against the policy method with no model argument, which is the hook intended for questions like "may this user create orders at all" - so the loaded $order never reaches the policy, and the answer is the same for every record in the table.
Route-model binding is what makes this convincing: $order is right there in the signature, already fetched, so the code appears to be reasoning about a specific order. Passing the instance is the entire fix, and $this->authorize('update', $order) is the form that is hard to write the wrong way.
Check Missing on a Sibling Action
// VULNERABLE - authorize() is called on update() but the bulk endpoint
// added later skips it entirely
class OrderController extends Controller
{
public function update(Request $request, Order $order)
{
$this->authorize('update', $order);
$order->update($request->validate(['status' => 'required|string']));
return response()->json($order);
}
public function bulkUpdate(Request $request)
{
// No authorize() call - added after update() and never given
// the same protection.
foreach ($request->input('orders', []) as $data) {
Order::find($data['id'])?->update(['status' => $data['status']]);
}
return response()->noContent();
}
}
Why this is vulnerable: Two actions reach the same rows and only one authorizes, so the control the data actually has is the weaker one. Laravel will not complain - a controller method without an authorize() call is an ordinary method.
The reliable fix is structural rather than per-action. authorizeResource() in the constructor binds a policy to every resource method at once, so a new action inherits the check instead of needing to remember it, and a policy method missing for a new ability fails closed. Where actions must stay bespoke, search by model rather than by route before closing the finding: the sibling is usually a bulk action, an export, or an older endpoint kept for a client.
Secure Patterns
Policy Combining Role Allowlist and Ownership
// SECURE - Policy combines an explicit role allowlist with an ownership check
use Illuminate\Auth\Access\Response;
use Illuminate\Support\Collection;
class OrderPolicy
{
private const ALLOWED_ROLES = ['admin', 'editor'];
public function update(User $user, Order $order): Response
{
if (!in_array($user->role, self::ALLOWED_ROLES, true)) {
return Response::denyAsNotFound();
}
if ($user->role === 'admin') {
return Response::allow();
}
// Ownership is checked against the loaded model, not the request.
// denyAsNotFound() rather than deny(): a 403 here would confirm the
// order exists, which route-model binding already answers with 404
// when it does not. Both outcomes have to be the same response.
return $order->user_id === $user->id
? Response::allow()
: Response::denyAsNotFound();
}
// Typed Collection, not array: the controller passes the result of
// ->get(), and every() is a Collection method. An `array` hint here is a
// TypeError on every call, and a real array would have no every() to call.
//
// $ids is passed in as well as $orders, because deciding the batch needs
// to know what was asked for and not only what came back.
public function bulkUpdate(User $user, Collection $orders, Collection $ids): Response
{
if (!in_array($user->role, self::ALLOWED_ROLES, true)) {
return Response::denyAsNotFound();
}
// Collection::every() returns true over an empty collection, so an
// empty batch would otherwise authorize and then update nothing.
if ($ids->isEmpty()) {
return Response::denyAsNotFound();
}
// whereIn silently omits IDs that do not exist, so the batch is only
// authorized when every ID asked for came back. Deciding it here,
// rather than aborting in the controller, is what keeps a missing ID
// and an unowned one on the same response path.
if ($orders->count() !== $ids->count()) {
return Response::denyAsNotFound();
}
if ($user->role === 'admin') {
return Response::allow();
}
// Every order in the batch must be owned by this user - one
// unauthorized ID anywhere in the batch denies the whole request.
return $orders->every(fn (Order $order) => $order->user_id === $user->id)
? Response::allow()
: Response::denyAsNotFound();
}
}
class OrderController extends Controller
{
public function update(Request $request, Order $order)
{
$this->authorize('update', $order);
$order->update($request->validate(['status' => 'required|string']));
return response()->json($order);
}
public function bulkUpdate(Request $request)
{
$ids = collect($request->input('orders', []))->pluck('id')->unique()->values();
$orders = Order::whereIn('id', $ids)->get();
// Every denial - disallowed role, empty batch, an ID that does not
// exist, an ID owned by someone else - leaves through this one call.
// The controller makes no authorization decision of its own, so there
// is no second response path for the caller to tell them apart by.
//
// Array form: the first element selects the Policy, the rest are
// passed to the method after $user - so bulkUpdate() receives
// $orders and then $ids.
$this->authorize('bulkUpdate', [Order::class, $orders, $ids]);
foreach ($request->input('orders', []) as $data) {
$orders->firstWhere('id', $data['id'])?->update(['status' => $data['status']]);
}
return response()->noContent();
}
}
Why this works: in_array($user->role, self::ALLOWED_ROLES, true) is an explicit allowlist - any role value not in the array is denied, including one introduced after the Policy was written. Ownership is compared against $order->user_id, a field loaded from the database, never from request input, so a valid role alone is no longer sufficient to act on someone else's order. Both update() and bulkUpdate() are Policy methods invoked through $this->authorize(), so the ownership rule for a batch of orders lives in the same reviewable place as the single-order rule instead of being reimplemented inline in the controller.
The Policy method's parameter type has to match what the controller hands it. $this->authorize('bulkUpdate', [Order::class, $orders, $ids]) uses Laravel's array form: the first element selects the Policy - needed here because no single model instance identifies it - and the remaining elements are passed to the method after $user. So bulkUpdate() receives whatever Order::whereIn(...)->get() returned, which is an Illuminate\Support\Collection, and typing that parameter array makes every call a TypeError before any authorization runs. A real array would not fix it either - every() is a Collection method, so the body would fail with Call to a member function every() on array. Both are fatal rather than silent, which is the redeeming part: this fails closed. It is still worth naming, because a Policy method that has never executed reads exactly like one that works.
Every denial has to leave by the same door, and for a batch that means the Policy decides the whole thing. Splitting the decision - abort_if($orders->count() !== $ids->count(), 404) in the controller, ownership in the Policy - reads as defence in depth and builds an existence oracle instead: a nonexistent ID gets 404 from the controller while an ID owned by someone else gets 403 from the Policy, so the pair of responses maps the table one request at a time. Because the controller check runs first, it answers even for a caller whose role the Policy would have rejected outright. Passing $ids into the Policy and returning Response::denyAsNotFound() from all four branches leaves exactly one response for every way the request can fail.
Response::denyAsNotFound() is what makes that practical on the single-resource route too. update() is reached through route-model binding, which already answers 404 for an ID that does not exist - so a Policy returning plain false, which Laravel renders as 403, distinguishes the two by itself with no help from the controller. Returning a Response rather than a bool from the Policy method is what lets one place decide both the outcome and the status.
Measured against the four ways a bulk request can fail, before and after:
split decision Policy decides
alice, owns every order in batch 204 204
alice, one order belongs to bob 403 404
alice, one ID does not exist 404 404
alice, empty batch 204 404
role not in the allowlist, real IDs 403 404
role not in the allowlist, fake IDs 404 404
The empty-batch row is the one to look at twice. Collection::every() returns true over an empty collection - the vacuous-truth default - so a batch naming nothing satisfied "every order is owned by this caller" and authorized, and the count comparison did not catch it either, because 0 !== 0 is false. It is not exploitable on its own, since there is nothing to update. It is the same defect that makes the missing-ID case matter, and it survives until something is asserted about the input rather than about what came back from the query.
Instance-Level Authorization Instead of Class-Level
// SECURE - authorize() is called with the loaded model instance, not the
// class name, so Policy::update() receives $order and can check ownership
class OrderController extends Controller
{
public function update(Request $request, Order $order)
{
$this->authorize('update', $order); // instance, not Order::class
$order->update($request->validate(['status' => 'required|string']));
return response()->json($order);
}
}
Why this works: passing the loaded $order instance to authorize() (rather than the Order::class string) means Laravel resolves and calls OrderPolicy::update(User $user, Order $order) with the actual model, giving the Policy method the data it needs to compare $order->user_id against the caller. A class-level check can only ever answer "can this user update orders in general," which is the wrong question for an endpoint that mutates one specific record.
Framework-Specific Guidance
Laravel Gate and Policy Registration
// SECURE - Policy registered centrally so every authorize()/can() call in
// the app resolves to the same logic, instead of ad hoc inline checks
class AuthServiceProvider extends ServiceProvider
{
protected $policies = [
Order::class => OrderPolicy::class,
];
public function boot(): void
{
$this->registerPolicies();
}
}
Why this works: registering the Policy in AuthServiceProvider (or relying on Laravel's model-name-based auto-discovery) means $this->authorize('update', $order), $request->user()->can('update', $order), and @can('update', $order) in a Blade view all resolve to the exact same OrderPolicy::update() method - there is only one place the ownership logic can drift out of sync, rather than a copy per call site.
Testing
- Role boundary: call the endpoint as a user with a role not in
ALLOWED_ROLES(a typo, a role introduced after the Policy was written) and confirm the request is denied, not a successful update. WithdenyAsNotFound()the expected status is404; assert the one the Policy actually returns rather than assuming403. - Cross-owner access: authenticate as one user and target another user's
OrderID through every action that touches it -show,update,destroy, andbulkUpdate- confirming each is independently denied. - Explicit deny on unmatched branches: unit-test the Policy method directly with role/ownership combinations that a missing
returnor a denylist comparison would previously have let through. - Batch boundary: for
bulkUpdate, include one order the caller does not own among several they do, and confirm the entire batch is denied rather than partially applied. Assert that no row changed, not only the status - a partial application returns the same response as a clean rejection. - Indistinguishable denials: send a batch naming an order that does not exist, a batch naming one owned by another user, and an empty batch, and assert all three return the identical status and body. A
404for one and a403for another is an existence oracle regardless of which status is which. - Use Laravel's
actingAs($user)in a feature test to exercise the real HTTP route and middleware stack, not just the Policy method in isolation.
Common Pitfalls
- Fixing the flagged action but not a sibling one: Adding the ownership check to
update()whiledestroy()orbulkUpdate()retains the original!=comparison or skipsauthorize()entirely, because each action's check was written independently. - Passing the model class instead of the instance to
authorize():$this->authorize('update', Order::class)only invokes a class-level check; the Policy method never receives the specific$order, so it cannot compare ownership no matter how the method itself is written. - Treating a missing
returnas a safe default: A Policy method with noreturnon some code path returnsnullin PHP, which Laravel's authorization gate treats as denied forauthorize()/can(). The hazard is custom middleware that inspects the result itself:nullis falsy under every loose comparison (null == falseistrue), so it is a strict test such as$result !== falsethat reads the missing branch as an allow - always return an explicitfalsefor denied branches. - Deciding part of the authorization in the controller: an
abort_if()on a missing ID before$this->authorize()splits one decision across two response paths - the controller answers404for an ID that does not exist and the Policy answers403for one owned by someone else, which together enumerate the table. It also runs before the Policy's role check, so it answers for callers the Policy would have refused outright. Give the Policy whatever it needs to decide the whole thing. - Resolving role or ownership from request input:
$request->input('role')or a hiddenowner_idform field can be set to anything by the client; the Policy must read$user->roleand the model's database field, never request data, for the values it compares.