From ca6dd57bf9ba38544a79a7652ec1f9dbc6207c02 Mon Sep 17 00:00:00 2001
From: Darko Gjorgjijoski <5760249+gdarko@users.noreply.github.com>
Date: Fri, 12 Jun 2026 11:02:15 +0200
Subject: [PATCH] fix(security): block ORDER BY SQL injection via orderByField
 (GHSA-cp8p) (#670)

v3 port. orderByField/orderBy were passed straight into Eloquent's orderBy()
in every model's scopeWhereOrder (and Invoice::scopeApplyFilters), allowing
arbitrary SQL in the ORDER BY clause.

Adds App\Support\SafeOrderBy::apply() (plain/table-qualified column identifier
only, asc/desc clamp) and routes all 10 model sort sinks through it. Aliased
sorts (e.g. estimates by customers.name) stay valid.

Adds unit tests for injection rejection, plain + aliased columns, direction clamp.
---
 app/Models/Customer.php         |  3 ++-
 app/Models/Estimate.php         |  3 ++-
 app/Models/Expense.php          |  3 ++-
 app/Models/FileDisk.php         |  3 ++-
 app/Models/Invoice.php          |  6 ++++--
 app/Models/Item.php             |  3 ++-
 app/Models/Payment.php          |  3 ++-
 app/Models/RecurringInvoice.php |  3 ++-
 app/Models/TaxType.php          |  3 ++-
 app/Models/User.php             |  3 ++-
 app/Support/SafeOrderBy.php     | 31 +++++++++++++++++++++++++++++++
 tests/Unit/SafeOrderByTest.php  | 32 ++++++++++++++++++++++++++++++++
 12 files changed, 85 insertions(+), 11 deletions(-)
 create mode 100644 app/Support/SafeOrderBy.php
 create mode 100644 tests/Unit/SafeOrderByTest.php

diff --git a/app/Models/Customer.php b/app/Models/Customer.php
index 349a688e..bcd2283d 100644
--- a/app/Models/Customer.php
+++ b/app/Models/Customer.php
@@ -3,6 +3,7 @@
 namespace App\Models;
 
 use App\Notifications\CustomerMailResetPasswordNotification;
+use App\Support\SafeOrderBy;
 use App\Traits\HasCustomFieldsTrait;
 use Carbon\Carbon;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
@@ -161,7 +162,7 @@ class Customer extends Authenticatable implements HasMedia
 
     public function scopeWhereOrder($query, $orderByField, $orderBy)
     {
-        $query->orderBy($orderByField, $orderBy);
+        SafeOrderBy::apply($query, $orderByField, $orderBy);
     }
 
     public function scopeWhereSearch($query, $search)
diff --git a/app/Models/Estimate.php b/app/Models/Estimate.php
index 2b05c54b..e2c4864c 100644
--- a/app/Models/Estimate.php
+++ b/app/Models/Estimate.php
@@ -5,6 +5,7 @@ namespace App\Models;
 use App\Services\Document\EstimateService;
 use App\Support\Pdf\PdfHtmlSanitizer;
 use App\Support\Pdf\PdfTemplateUtils;
+use App\Support\SafeOrderBy;
 use App\Traits\GeneratesPdfTrait;
 use App\Traits\HasCustomFieldsTrait;
 use Carbon\Carbon;
@@ -191,7 +192,7 @@ class Estimate extends Model implements HasMedia
 
     public function scopeWhereOrder($query, $orderByField, $orderBy)
     {
-        $query->orderBy($orderByField, $orderBy);
+        SafeOrderBy::apply($query, $orderByField, $orderBy);
     }
 
     public function scopeWhereCompany($query)
diff --git a/app/Models/Expense.php b/app/Models/Expense.php
index b9209d46..4b135584 100644
--- a/app/Models/Expense.php
+++ b/app/Models/Expense.php
@@ -2,6 +2,7 @@
 
 namespace App\Models;
 
+use App\Support\SafeOrderBy;
 use App\Traits\HasCustomFieldsTrait;
 use Carbon\Carbon;
 use Illuminate\Contracts\Pagination\LengthAwarePaginator;
@@ -212,7 +213,7 @@ class Expense extends Model implements HasMedia
 
     public function scopeWhereOrder(Builder $query, string $orderByField, string $orderBy): void
     {
-        $query->orderBy($orderByField, $orderBy);
+        SafeOrderBy::apply($query, $orderByField, $orderBy);
     }
 
     public function scopeWhereCompany(Builder $query): void
diff --git a/app/Models/FileDisk.php b/app/Models/FileDisk.php
index 653c9500..8d26caae 100644
--- a/app/Models/FileDisk.php
+++ b/app/Models/FileDisk.php
@@ -3,6 +3,7 @@
 namespace App\Models;
 
 use App\Services\Storage\FileDiskService;
+use App\Support\SafeOrderBy;
 use Carbon\Carbon;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
@@ -49,7 +50,7 @@ class FileDisk extends Model
 
     public function scopeWhereOrder($query, $orderByField, $orderBy)
     {
-        $query->orderBy($orderByField, $orderBy);
+        SafeOrderBy::apply($query, $orderByField, $orderBy);
     }
 
     public function scopeFileDisksBetween($query, $start, $end)
diff --git a/app/Models/Invoice.php b/app/Models/Invoice.php
index 48089932..3b9b0262 100644
--- a/app/Models/Invoice.php
+++ b/app/Models/Invoice.php
@@ -5,6 +5,7 @@ namespace App\Models;
 use App\Services\Document\InvoiceService;
 use App\Support\Pdf\PdfHtmlSanitizer;
 use App\Support\Pdf\PdfTemplateUtils;
+use App\Support\SafeOrderBy;
 use App\Traits\GeneratesPdfTrait;
 use App\Traits\HasCustomFieldsTrait;
 use Carbon\Carbon;
@@ -257,7 +258,7 @@ class Invoice extends Model implements HasMedia
 
     public function scopeWhereOrder($query, $orderByField, $orderBy)
     {
-        $query->orderBy($orderByField, $orderBy);
+        SafeOrderBy::apply($query, $orderByField, $orderBy);
     }
 
     public function scopeApplyFilters($query, array $filters)
@@ -286,7 +287,8 @@ class Invoice extends Model implements HasMedia
             $query->where('customer_id', $customerId);
         })->when($filters['orderByField'] ?? null, function ($query, $orderByField) use ($filters) {
             $orderBy = $filters['orderBy'] ?? 'desc';
-            $query->orderBy($orderByField, $orderBy);
+
+            SafeOrderBy::apply($query, $orderByField, $orderBy);
         }, function ($query) {
             $query->orderBy('sequence_number', 'desc');
         });
diff --git a/app/Models/Item.php b/app/Models/Item.php
index fc90d65c..4ce6cae2 100644
--- a/app/Models/Item.php
+++ b/app/Models/Item.php
@@ -2,6 +2,7 @@
 
 namespace App\Models;
 
+use App\Support\SafeOrderBy;
 use Carbon\Carbon;
 use Illuminate\Contracts\Pagination\LengthAwarePaginator;
 use Illuminate\Database\Eloquent\Builder;
@@ -65,7 +66,7 @@ class Item extends Model
 
     public function scopeWhereOrder(Builder $query, string $orderByField, string $orderBy): void
     {
-        $query->orderBy($orderByField, $orderBy);
+        SafeOrderBy::apply($query, $orderByField, $orderBy);
     }
 
     public function scopeWhereItem(Builder $query, int $item_id): void
diff --git a/app/Models/Payment.php b/app/Models/Payment.php
index 30c4aaa9..add37c5e 100644
--- a/app/Models/Payment.php
+++ b/app/Models/Payment.php
@@ -5,6 +5,7 @@ namespace App\Models;
 use App\Jobs\GeneratePaymentPdfJob;
 use App\Services\Document\PaymentService;
 use App\Support\Pdf\PdfHtmlSanitizer;
+use App\Support\SafeOrderBy;
 use App\Traits\GeneratesPdfTrait;
 use App\Traits\HasCustomFieldsTrait;
 use Carbon\Carbon;
@@ -194,7 +195,7 @@ class Payment extends Model implements HasMedia
 
     public function scopeWhereOrder($query, $orderByField, $orderBy)
     {
-        $query->orderBy($orderByField, $orderBy);
+        SafeOrderBy::apply($query, $orderByField, $orderBy);
     }
 
     public function scopeWherePayment($query, $payment_id)
diff --git a/app/Models/RecurringInvoice.php b/app/Models/RecurringInvoice.php
index 39b19b20..2b8297ab 100644
--- a/app/Models/RecurringInvoice.php
+++ b/app/Models/RecurringInvoice.php
@@ -2,6 +2,7 @@
 
 namespace App\Models;
 
+use App\Support\SafeOrderBy;
 use App\Traits\HasCustomFieldsTrait;
 use Carbon\Carbon;
 use Cron;
@@ -129,7 +130,7 @@ class RecurringInvoice extends Model
 
     public function scopeWhereOrder($query, $orderByField, $orderBy)
     {
-        $query->orderBy($orderByField, $orderBy);
+        SafeOrderBy::apply($query, $orderByField, $orderBy);
     }
 
     public function scopeWhereStatus($query, $status)
diff --git a/app/Models/TaxType.php b/app/Models/TaxType.php
index 961ccee1..b2d6adc7 100644
--- a/app/Models/TaxType.php
+++ b/app/Models/TaxType.php
@@ -2,6 +2,7 @@
 
 namespace App\Models;
 
+use App\Support\SafeOrderBy;
 use Illuminate\Contracts\Pagination\LengthAwarePaginator;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
@@ -76,7 +77,7 @@ class TaxType extends Model
 
     public function scopeWhereOrder(Builder $query, string $orderByField, string $orderBy): void
     {
-        $query->orderBy($orderByField, $orderBy);
+        SafeOrderBy::apply($query, $orderByField, $orderBy);
     }
 
     public function scopeWhereSearch(Builder $query, string $search): void
diff --git a/app/Models/User.php b/app/Models/User.php
index 5ec2414e..d002bb12 100644
--- a/app/Models/User.php
+++ b/app/Models/User.php
@@ -3,6 +3,7 @@
 namespace App\Models;
 
 use App\Notifications\MailResetPasswordNotification;
+use App\Support\SafeOrderBy;
 use App\Traits\HasCustomFieldsTrait;
 use Carbon\Carbon;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
@@ -194,7 +195,7 @@ class User extends Authenticatable implements HasMedia
 
     public function scopeWhereOrder($query, $orderByField, $orderBy)
     {
-        $query->orderBy($orderByField, $orderBy);
+        SafeOrderBy::apply($query, $orderByField, $orderBy);
     }
 
     public function scopeWhereSearch($query, $search)
diff --git a/app/Support/SafeOrderBy.php b/app/Support/SafeOrderBy.php
new file mode 100644
index 00000000..f0793e0d
--- /dev/null
+++ b/app/Support/SafeOrderBy.php
@@ -0,0 +1,31 @@
+<?php
+
+namespace App\Support;
+
+class SafeOrderBy
+{
+    /**
+     * Apply a safe ORDER BY clause.
+     *
+     * The orderByField / orderBy values originate from user-supplied query
+     * parameters and were previously passed straight into Eloquent's orderBy(),
+     * allowing arbitrary SQL expressions in the ORDER BY clause (boolean-based
+     * blind injection). Only a plain, optionally table-qualified column
+     * identifier is accepted as the sort target; anything containing SQL syntax
+     * (parentheses, whitespace, sub-selects, ...) falls back to a safe default.
+     * The direction is clamped to asc/desc. Column aliases such as a joined
+     * "customers.name" remain valid, so legitimate sorts are unaffected.
+     */
+    public static function apply($query, $orderByField, $orderBy = 'desc', string $default = 'created_at')
+    {
+        $direction = strtolower((string) $orderBy) === 'asc' ? 'asc' : 'desc';
+
+        $field = is_string($orderByField) ? $orderByField : '';
+
+        if (! preg_match('/^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)?$/', $field)) {
+            $field = $default;
+        }
+
+        return $query->orderBy($field, $direction);
+    }
+}
diff --git a/tests/Unit/SafeOrderByTest.php b/tests/Unit/SafeOrderByTest.php
new file mode 100644
index 00000000..8734dc9e
--- /dev/null
+++ b/tests/Unit/SafeOrderByTest.php
@@ -0,0 +1,32 @@
+<?php
+
+use App\Support\SafeOrderBy;
+use Illuminate\Support\Facades\DB;
+
+test('rejects sql expressions in the order-by field', function () {
+    $sql = strtolower(
+        SafeOrderBy::apply(DB::table('invoices'), '(CASE WHEN (SELECT 1)=1 THEN id ELSE total END)', 'asc')->toSql()
+    );
+
+    expect($sql)->toContain('order by')
+        ->and($sql)->not->toContain('case')
+        ->and($sql)->not->toContain('select 1');
+});
+
+test('allows a plain column', function () {
+    $sql = strtolower(SafeOrderBy::apply(DB::table('invoices'), 'invoice_date', 'asc')->toSql());
+
+    expect($sql)->toContain('order by')->and($sql)->toContain('invoice_date');
+});
+
+test('allows a table-qualified column so joined/aliased sorts keep working', function () {
+    $sql = strtolower(SafeOrderBy::apply(DB::table('estimates'), 'customers.name', 'asc')->toSql());
+
+    expect($sql)->toContain('customers')->and($sql)->toContain('name');
+});
+
+test('clamps the direction to asc/desc', function () {
+    $sql = strtolower(SafeOrderBy::apply(DB::table('invoices'), 'id', 'asc); drop table users; --')->toSql());
+
+    expect($sql)->toContain('order by')->and($sql)->not->toContain('drop table');
+});