-
Notifications
You must be signed in to change notification settings - Fork 183
Description
Summary
Security audit of xshop identified 6 authorization vulnerabilities across customer and admin interfaces.
Findings
1. CRITICAL: Ticket IDOR — Any Customer Views/Replies to Any Customer's Tickets
File: app/Http/Controllers/CustomerController.php:267-289
showTicket() and ticketAnswer() accept a Ticket model via route model binding but perform zero ownership verification. Any authenticated customer can view any ticket and inject replies.
Comparison: invoice() (line 129-133) properly checks $invoice->customer_id != auth('customer')->id(). addressUpdate() and addressDestroy() also check ownership.
Fix: Add if ($ticket->customer_id != auth('customer')->user()->id) abort(403);
2. HIGH: Privilege Escalation via Bulk Role Change
File: app/Http/Controllers/Admin/UserController.php:154-160
save() (line 39-44) guards DEVELOPER role changes. But bulk() method's 'role' action has no such guards — an ADMIN can change any user to DEVELOPER.
Fix: Add the same DEVELOPER role guards from save() to the bulk() method's role case.
3. HIGH: Admin Panel Accessible to All Authenticated Users
File: routes/web.php:14-16, app/Http/Middleware/Acl.php:22-46
Admin routes protected only by ['middleware' => ['auth']]. ACL middleware only checks when isset($requestPath[2]) — 2-segment routes bypass checks.
Fix: Add role-based middleware to the admin route group (e.g., ->middleware(['auth', 'role:admin|developer'])).
4. HIGH: Invoice Payment Without Authentication
File: app/Http/Controllers/ClientController.php:880-916
pay($hash) processes payments without auth or ownership checks.
Comparison: invoice() in CustomerController checks both auth and ownership.
Fix: Add authentication and ownership verification.
5. HIGH: Address IDOR in Checkout
File: app/Http/Controllers/CardController.php:168-176
Checkout validates address_id only as 'exists:addresses,id' without ownership check. Leaks other customers' addresses.
Comparison: addressUpdate() and addressDestroy() properly check ownership.
Fix: Change validation to verify address belongs to authenticated customer.
6. MEDIUM: Premium Attachment Download Bypass
Attachment downloads lack auth or premium verification.
Impact
Cross-customer data access, privilege escalation to developer role, unauthenticated payment triggering.
Disclosure
This report was generated during a systematic open-source security audit. Happy to discuss findings or assist with fixes.