Skip to content

Commit 13d047b

Browse files
Merge commit from fork
The public `sortDirection` Livewire property was interpolated verbatim into raw ORDER BY clauses, allowing SQL injection through POST /livewire/update. A direction keyword cannot be a bound parameter, so it must be interpolated as a literal — the fix restricts it to a strict asc/desc allowlist via a new `Sql::sanitizeSortDirection()` helper, applied both at the source (the property) and at the two raw-SQL sinks: - naturalSort: ColumnRawQueries resolves {sortDirection} to a sanitized value (the advisory path; fires even when sortField is empty). - sortUsing/sortCallback: Database\Pipelines\Sorting sanitizes the direction before handing it to developer closures that may build orderByRaw. - source: sortBy(), updatedSortDirection() and Persist::restoreState() now normalize the property (covers the Livewire update vector and tampered persisted state). Laravel's orderBy() already validates the direction, so the plain (non-raw) paths were not injectable; normalizing there also prevents a 500 on tampered input.
1 parent d5ec733 commit 13d047b

8 files changed

Lines changed: 168 additions & 2 deletions

File tree

src/Concerns/Persist.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Exception;
66
use Illuminate\Support\Facades\{Cache, Cookie, Session};
7+
use PowerComponents\LivewirePowerGrid\DataSource\Support\Sql;
78
use PowerComponents\LivewirePowerGrid\PowerGridComponent;
89
use Psr\SimpleCache\InvalidArgumentException;
910

@@ -99,7 +100,7 @@ private function restoreState(): void
99100

100101
if (in_array('sorting', $this->persist) && array_key_exists('sortField', $state)) {
101102
$this->sortField = $state['sortField'];
102-
$this->sortDirection = $state['sortDirection'];
103+
$this->sortDirection = Sql::sanitizeSortDirection($state['sortDirection'] ?? null);
103104
$this->sortArray = $state['sortArray'];
104105
$this->multiSort = $state['multiSort'];
105106
}

src/Concerns/Sorting.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Exception;
66
use PowerComponents\LivewirePowerGrid\Column;
7+
use PowerComponents\LivewirePowerGrid\DataSource\Support\Sql;
78
use stdClass;
89

910
trait Sorting
@@ -27,7 +28,7 @@ public function sortBy(string $field, string $direction = 'asc'): void
2728
return;
2829
}
2930

30-
$this->sortDirection = $this->sortField === $field ? $this->reverseSort() : $direction;
31+
$this->sortDirection = $this->sortField === $field ? $this->reverseSort() : Sql::sanitizeSortDirection($direction);
3132

3233
$this->sortField = $field;
3334

@@ -102,6 +103,8 @@ public function sortIcon(string $field): string
102103

103104
public function updatedSortDirection(): void
104105
{
106+
$this->sortDirection = Sql::sanitizeSortDirection($this->sortDirection);
107+
105108
if ($this->hasLazyEnabled) {
106109
data_set($this->setUp, 'lazy.items', 0);
107110

src/DataSource/Processors/Database/Pipelines/ColumnRawQueries.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
77
use Illuminate\Database\Eloquent\Relations\MorphToMany;
88
use Illuminate\Database\Query\Builder as QueryBuilder;
9+
use PowerComponents\LivewirePowerGrid\DataSource\Support\Sql;
910
use PowerComponents\LivewirePowerGrid\PowerGridComponent;
1011

1112
class ColumnRawQueries
@@ -62,6 +63,13 @@ private function resolvePlaceholders(?string $sql): ?string
6263
return preg_replace_callback('/\{(\w+)\}/', function ($matches) {
6364
$property = trim($matches[1]);
6465

66+
// The sort direction is interpolated verbatim into a raw ORDER BY
67+
// clause, so it must be restricted to an "asc"/"desc" allowlist to
68+
// prevent SQL injection through the public sortDirection property.
69+
if ($property === 'sortDirection') {
70+
return Sql::sanitizeSortDirection($this->component->sortDirection);
71+
}
72+
6573
return data_get($this->component, $property, '');
6674
}, $sql);
6775
}

src/DataSource/Processors/Database/Pipelines/Sorting.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
77
use Illuminate\Database\Eloquent\Relations\MorphToMany;
88
use Illuminate\Database\Query\Builder as QueryBuilder;
9+
use PowerComponents\LivewirePowerGrid\DataSource\Support\Sql;
910
use PowerComponents\LivewirePowerGrid\PowerGridComponent;
1011

1112
class Sorting
@@ -31,6 +32,10 @@ public function handle(mixed $query, Closure $next): mixed
3132

3233
private function applySingleSort(EloquentBuilder|MorphToMany|QueryBuilder $query, string $sortField, string $direction): void
3334
{
35+
// A sort callback may interpolate the direction into a raw ORDER BY
36+
// (e.g. orderByRaw), so it must be restricted to "asc"/"desc" here.
37+
$direction = Sql::sanitizeSortDirection($direction);
38+
3439
$sortCallback = $this->component->getSortCallback($sortField);
3540

3641
if ($sortCallback !== null) {
@@ -45,6 +50,8 @@ private function applySingleSort(EloquentBuilder|MorphToMany|QueryBuilder $query
4550
private function applyMultipleSort(EloquentBuilder|MorphToMany|QueryBuilder $results): void
4651
{
4752
foreach ($this->component->sortArray as $sortField => $direction) {
53+
$direction = Sql::sanitizeSortDirection($direction);
54+
4855
$sortCallback = $this->component->getSortCallback($sortField);
4956

5057
if ($sortCallback !== null) {

src/DataSource/Support/Sql.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,20 @@ public static function isValidSortFieldType(?string $sortFieldType): bool
114114
return in_array($sortFieldType, self::$sortStringNumberTypes);
115115
}
116116

117+
/**
118+
* Restrict a sort direction to a strict "asc"/"desc" allowlist.
119+
*
120+
* The direction keyword is interpolated verbatim into raw ORDER BY clauses
121+
* (it cannot be a bound parameter), so anything outside the allowlist must
122+
* be normalized to prevent SQL injection. Defaults to "asc".
123+
*/
124+
public static function sanitizeSortDirection(?string $direction): string
125+
{
126+
$direction = strtolower(trim((string) $direction));
127+
128+
return in_array($direction, ['asc', 'desc'], true) ? $direction : 'asc';
129+
}
130+
117131
/**
118132
* @throws Exception
119133
*/
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php
2+
3+
namespace PowerComponents\LivewirePowerGrid\Tests\Concerns\Components;
4+
5+
use Illuminate\Database\Eloquent\Builder;
6+
use PowerComponents\LivewirePowerGrid\{Column, Facades\PowerGrid, PowerGridComponent, PowerGridFields};
7+
use PowerComponents\LivewirePowerGrid\Tests\Concerns\Models\Dish;
8+
9+
class DishesNaturalSortTable extends PowerGridComponent
10+
{
11+
public string $tableName = 'dishes-natural-sort-table';
12+
13+
public bool $ignoreTablePrefix = true;
14+
15+
public function setUp(): array
16+
{
17+
return [
18+
PowerGrid::header()
19+
->showSearchInput(),
20+
21+
PowerGrid::footer()
22+
->showPerPage()
23+
->showRecordCount(),
24+
];
25+
}
26+
27+
public function datasource(): Builder
28+
{
29+
return Dish::query();
30+
}
31+
32+
public function fields(): PowerGridFields
33+
{
34+
return PowerGrid::fields()
35+
->add('id')
36+
->add('name')
37+
->add('stored_at');
38+
}
39+
40+
public function columns(): array
41+
{
42+
return [
43+
Column::make('Id', 'id')
44+
->sortable(),
45+
46+
Column::make('Name', 'name')
47+
->sortable(),
48+
49+
Column::make('Stored at', 'stored_at')
50+
->naturalSort(true),
51+
52+
Column::action('Action'),
53+
];
54+
}
55+
56+
public function setTestThemeClass(string $themeClass): void
57+
{
58+
config(['livewire-powergrid.theme' => $themeClass]);
59+
}
60+
}

tests/Feature/Helpers/SqlSupportTest.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,18 @@
5252
[['db' => 'sqlsrv', 'version' => '20.80', 'expected' => "CAST(SUBSTRING(field, PATINDEX('%[a-z]%', field), LEN(field)-PATINDEX('%[a-z]%', field)) AS INT) {sortDirection}"]],
5353
[['db' => 'unsupported-db', 'version' => '29.00.00', 'expected' => 'field+0 {sortDirection}']],
5454
]);
55+
56+
it('restricts the sort direction to an asc/desc allowlist', function (?string $input, string $expected) {
57+
expect(Sql::sanitizeSortDirection($input))->toBe($expected);
58+
})->with([
59+
'asc' => ['asc', 'asc'],
60+
'desc' => ['desc', 'desc'],
61+
'uppercase asc' => ['ASC', 'asc'],
62+
'uppercase desc' => ['DESC', 'desc'],
63+
'trims surrounding space' => [' desc ', 'desc'],
64+
'empty string' => ['', 'asc'],
65+
'null' => [null, 'asc'],
66+
'unknown keyword' => ['foo', 'asc'],
67+
'time-based injection' => ['asc, (SELECT SLEEP(3))', 'asc'],
68+
'quote break-out' => ["asc'; DROP TABLE users; --", 'asc'],
69+
]);
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
use PowerComponents\LivewirePowerGrid\DataSource\Processors\Database\Pipelines\{ColumnRawQueries, Sorting};
4+
use PowerComponents\LivewirePowerGrid\Tests\Concerns\Components\{DishesCustomSortTable, DishesNaturalSortTable};
5+
use PowerComponents\LivewirePowerGrid\Tests\Concerns\Models\Dish;
6+
7+
use function PowerComponents\LivewirePowerGrid\Tests\Plugins\livewire;
8+
9+
/*
10+
| Regression tests for GHSA-7fgc-3h6c-698r — SQL injection through the public
11+
| sortDirection Livewire property. The direction reaches raw ORDER BY clauses
12+
| through two paths (naturalSort and sortUsing callbacks); both must be
13+
| restricted to an asc/desc allowlist.
14+
*/
15+
16+
$payload = 'asc, (SELECT SLEEP(3))';
17+
18+
it('neutralizes SQL injection through sortDirection on a naturalSort column', function () use ($payload) {
19+
$component = new DishesNaturalSortTable();
20+
21+
// The advisory bypass: an empty sortField skips the Sorting pipeline, but
22+
// ColumnRawQueries still applies the naturalSort orderByRaw unconditionally.
23+
$component->sortField = '';
24+
$component->sortDirection = $payload;
25+
26+
$query = Dish::query();
27+
28+
(new ColumnRawQueries($component))->handle($query, fn ($q) => $q);
29+
30+
expect($query->toSql())
31+
->not->toContain('SLEEP')
32+
->toContain(' asc');
33+
});
34+
35+
it('neutralizes SQL injection through sortDirection in a custom sort callback', function () use ($payload) {
36+
$component = new DishesCustomSortTable();
37+
38+
// The "price" column uses sortUsing() with orderByRaw("... {$direction}").
39+
$component->sortField = 'price';
40+
$component->sortDirection = $payload;
41+
42+
$query = Dish::query();
43+
44+
(new Sorting($component))->handle($query, fn ($q) => $q);
45+
46+
expect($query->toSql())->not->toContain('SLEEP');
47+
});
48+
49+
it('normalizes a tampered sortDirection to asc through the Livewire lifecycle', function () use ($payload) {
50+
// Reproduces the exact advisory request: sortField="" + a malicious
51+
// sortDirection sent via /livewire/update. The updatedSortDirection hook
52+
// must normalize it, and the render (which runs the naturalSort query)
53+
// must not fail with a SQL error.
54+
livewire(DishesNaturalSortTable::class)
55+
->set('sortField', '')
56+
->set('sortDirection', $payload)
57+
->assertSet('sortDirection', 'asc');
58+
});

0 commit comments

Comments
 (0)