fix(security): block ORDER BY SQL injection via orderByField (GHSA-cp8p) (#663)

The orderByField/orderBy query params were passed straight into Eloquent's
orderBy() in every model's scopeWhereOrder (and Invoice::scopeApplyFilters),
allowing arbitrary SQL in the ORDER BY clause (boolean-based blind injection).

Adds App\Support\SafeOrderBy::apply() which only accepts a plain, optionally
table-qualified column identifier as the sort target (rejecting expressions,
sub-selects, etc.) and clamps the direction to asc/desc. Routed all 10 model
sort sinks through it. Table-qualified columns stay valid, so joined/aliased
sorts (e.g. estimates by customers.name) are unaffected.

Adds unit tests covering injection rejection, plain + aliased columns, and
direction clamping.
This commit is contained in:
Darko Gjorgjijoski
2026-06-12 09:18:29 +02:00
committed by GitHub
parent 5839d8385d
commit e92b08ef6a
12 changed files with 85 additions and 11 deletions

View File

@@ -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;
@@ -269,7 +270,7 @@ class Customer extends Authenticatable implements HasMedia
public function scopeWhereOrder($query, $orderByField, $orderBy)
{
$query->orderBy($orderByField, $orderBy);
return SafeOrderBy::apply($query, $orderByField, $orderBy);
}
public function scopeWhereSearch($query, $search)

View File

@@ -8,6 +8,7 @@ use App\Facades\PDF;
use App\Mail\SendEstimateMail;
use App\Services\SerialNumberFormatter;
use App\Space\PdfTemplateUtils;
use App\Support\SafeOrderBy;
use App\Traits\GeneratesPdfTrait;
use App\Traits\HasCustomFieldsTrait;
use Carbon\Carbon;
@@ -194,7 +195,7 @@ class Estimate extends Model implements HasMedia
public function scopeWhereOrder($query, $orderByField, $orderBy)
{
$query->orderBy($orderByField, $orderBy);
return SafeOrderBy::apply($query, $orderByField, $orderBy);
}
public function scopeWhereCompany($query)

View File

@@ -2,6 +2,7 @@
namespace App\Models;
use App\Support\SafeOrderBy;
use App\Traits\HasCustomFieldsTrait;
use Carbon\Carbon;
use Illuminate\Database\Eloquent\Factories\HasFactory;
@@ -200,7 +201,7 @@ class Expense extends Model implements HasMedia
public function scopeWhereOrder($query, $orderByField, $orderBy)
{
$query->orderBy($orderByField, $orderBy);
return SafeOrderBy::apply($query, $orderByField, $orderBy);
}
public function scopeWhereCompany($query)

View File

@@ -2,6 +2,7 @@
namespace App\Models;
use App\Support\SafeOrderBy;
use Carbon\Carbon;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;
@@ -32,7 +33,7 @@ class FileDisk extends Model
public function scopeWhereOrder($query, $orderByField, $orderBy)
{
$query->orderBy($orderByField, $orderBy);
return SafeOrderBy::apply($query, $orderByField, $orderBy);
}
public function scopeFileDisksBetween($query, $start, $end)

View File

@@ -8,6 +8,7 @@ use App\Facades\PDF;
use App\Mail\SendInvoiceMail;
use App\Services\SerialNumberFormatter;
use App\Space\PdfTemplateUtils;
use App\Support\SafeOrderBy;
use App\Traits\GeneratesPdfTrait;
use App\Traits\HasCustomFieldsTrait;
use Carbon\Carbon;
@@ -259,7 +260,7 @@ class Invoice extends Model implements HasMedia
public function scopeWhereOrder($query, $orderByField, $orderBy)
{
$query->orderBy($orderByField, $orderBy);
return SafeOrderBy::apply($query, $orderByField, $orderBy);
}
public function scopeApplyFilters($query, array $filters)
@@ -288,7 +289,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);
return SafeOrderBy::apply($query, $orderByField, $orderBy);
}, function ($query) {
$query->orderBy('sequence_number', 'desc');
});

View File

@@ -2,6 +2,7 @@
namespace App\Models;
use App\Support\SafeOrderBy;
use Carbon\Carbon;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;
@@ -63,7 +64,7 @@ class Item extends Model
public function scopeWhereOrder($query, $orderByField, $orderBy)
{
$query->orderBy($orderByField, $orderBy);
return SafeOrderBy::apply($query, $orderByField, $orderBy);
}
public function scopeWhereItem($query, $item_id)

View File

@@ -6,6 +6,7 @@ use App\Facades\Hashids;
use App\Jobs\GeneratePaymentPdfJob;
use App\Mail\SendPaymentMail;
use App\Services\SerialNumberFormatter;
use App\Support\SafeOrderBy;
use App\Traits\GeneratesPdfTrait;
use App\Traits\HasCustomFieldsTrait;
use Barryvdh\DomPDF\Facade\Pdf as PDF;
@@ -356,7 +357,7 @@ class Payment extends Model implements HasMedia
public function scopeWhereOrder($query, $orderByField, $orderBy)
{
$query->orderBy($orderByField, $orderBy);
return SafeOrderBy::apply($query, $orderByField, $orderBy);
}
public function scopeWherePayment($query, $payment_id)

View File

@@ -5,6 +5,7 @@ namespace App\Models;
use App\Facades\Hashids;
use App\Http\Requests\RecurringInvoiceRequest;
use App\Services\SerialNumberFormatter;
use App\Support\SafeOrderBy;
use App\Traits\HasCustomFieldsTrait;
use Carbon\Carbon;
use Cron;
@@ -132,7 +133,7 @@ class RecurringInvoice extends Model
public function scopeWhereOrder($query, $orderByField, $orderBy)
{
$query->orderBy($orderByField, $orderBy);
return SafeOrderBy::apply($query, $orderByField, $orderBy);
}
public function scopeWhereStatus($query, $status)

View File

@@ -2,6 +2,7 @@
namespace App\Models;
use App\Support\SafeOrderBy;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo;
@@ -73,7 +74,7 @@ class TaxType extends Model
public function scopeWhereOrder($query, $orderByField, $orderBy)
{
$query->orderBy($orderByField, $orderBy);
return SafeOrderBy::apply($query, $orderByField, $orderBy);
}
public function scopeWhereSearch($query, $search)

View File

@@ -4,6 +4,7 @@ namespace App\Models;
use App\Http\Requests\UserRequest;
use App\Notifications\MailResetPasswordNotification;
use App\Support\SafeOrderBy;
use App\Traits\HasCustomFieldsTrait;
use Carbon\Carbon;
use Illuminate\Database\Eloquent\Factories\HasFactory;
@@ -179,7 +180,7 @@ class User extends Authenticatable implements HasMedia
public function scopeWhereOrder($query, $orderByField, $orderBy)
{
$query->orderBy($orderByField, $orderBy);
return SafeOrderBy::apply($query, $orderByField, $orderBy);
}
public function scopeWhereSearch($query, $search)

View File

@@ -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);
}
}

View File

@@ -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');
});