Multiple cross-company IDOR vulnerabilities in multi-company mode

[lighthousekeeper1212]

Summary

Multiple cross-company IDOR (Insecure Direct Object Reference) vulnerabilities exist in Crater's multi-company mode. The most critical: all 7 bulk delete endpoints accept IDs from any company, allowing Company A to delete Company B's data.

Findings

1. HIGH - Cross-Company Bulk Delete (ALL 7 Resource Types)

All bulk delete endpoints accept IDs, validate only existence globally (no company filter), and authorize only class-level ability.

Affected: invoices/delete, estimates/delete, payments/delete, customers/delete, items/delete, expenses/delete, recurring-invoices/delete

Example (InvoicesController.php:101):

public function delete(DeleteInvoiceRequest $request)
{
    $this->authorize('delete multiple invoices');  // Class-level only
    Invoice::deleteInvoices($request->ids);  // Global find by ID
}

Validation (DeleteInvoiceRequest.php:33):

Rule::exists('invoices', 'id'),  // No company_id filter

2. MEDIUM - CustomerPolicy Missing hasCompany() Check

CustomerPolicy is the ONLY policy missing $user->hasCompany(). All others (Invoice, Estimate, etc.) include it.

CustomerPolicy.php:38: BouncerFacade::can('view-customer', $customer) — missing && $user->hasCompany($customer->company_id)

Compare InvoicePolicy.php:38: Correctly has && $user->hasCompany($invoice->company_id)

3. MEDIUM - Inverted Logic in Ownership Transfer

CompaniesController.php:64: if ($user->hasCompany($company->id)) returns error "User does not belongs to this company". But hasCompany() returning true means they DO belong. Transfer succeeds for non-members, fails for members.

4. MEDIUM - User Bulk Delete Without Company Scoping

UsersController::delete() uses self::find($id) globally without company filter.

5. MEDIUM - Cross-Company PDF Access

PDF controllers (InvoicePdfController, EstimatePdfController, PaymentPdfController) have no authorization check. Any authenticated user from any company can access PDFs via unique_hash.

Recommended Fix

1. Add ->where('company_id', request()->header('company')) to all bulk delete validators
2. Add $user->hasCompany($customer->company_id) to CustomerPolicy
3. Fix inverted logic: if (! $user->hasCompany(...))
4. Scope user deletion to current company
5. Add $this->authorize('view', $resource) to PDF controllers