From 4308a93cc81a75227bcbfc1abd4aaf5e21ef21ee Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Mon, 21 Nov 2022 13:01:32 +1300 Subject: [PATCH] [CVE-2022-38148] Validate SortColumn exists --- .../GridField/GridFieldSortableHeader.php | 11 ++++++ .../GridField/GridFieldSortableHeaderTest.php | 36 +++++++++++++++---- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/Forms/GridField/GridFieldSortableHeader.php b/src/Forms/GridField/GridFieldSortableHeader.php index c9e9e03e5..61cf81ca9 100644 --- a/src/Forms/GridField/GridFieldSortableHeader.php +++ b/src/Forms/GridField/GridFieldSortableHeader.php @@ -11,6 +11,7 @@ use SilverStripe\ORM\DataObject; use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; use LogicException; +use SilverStripe\Core\Injector\Injector; /** * GridFieldSortableHeader adds column headers to a {@link GridField} that can @@ -271,6 +272,16 @@ class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataM return $dataList; } + // Prevent SQL Injection by validating that SortColumn exists + /** @var GridFieldDataColumns $columns */ + $columns = $gridField->getConfig()->getComponentByType(GridFieldDataColumns::class); + $fields = $columns->getDisplayFields($gridField); + if (!array_key_exists($state->SortColumn, $fields) && + !in_array($state->SortColumn, $this->getFieldSorting()) + ) { + throw new LogicException('Invalid SortColumn: ' . $state->SortColumn); + } + return $dataList->sort($state->SortColumn, $state->SortDirection('asc')); } diff --git a/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php b/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php index 5973f1589..5a48de8ec 100644 --- a/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php +++ b/tests/php/Forms/GridField/GridFieldSortableHeaderTest.php @@ -71,17 +71,18 @@ class GridFieldSortableHeaderTest extends SapphireTest $list = Team::get()->filter([ 'ClassName' => Team::class ]); $config = new GridFieldConfig_RecordEditor(); $gridField = new GridField('testfield', 'testfield', $list, $config); + $component = $gridField->getConfig()->getComponentByType(GridFieldSortableHeader::class); // Test normal sorting + $component->setFieldSorting(['Name' => 'City']); $state = $gridField->State->GridFieldSortableHeader; $state->SortColumn = 'City'; $state->SortDirection = 'asc'; - $compontent = $gridField->getConfig()->getComponentByType(GridFieldSortableHeader::class); - $listA = $compontent->getManipulatedData($gridField, $list); + $listA = $component->getManipulatedData($gridField, $list); $state->SortDirection = 'desc'; - $listB = $compontent->getManipulatedData($gridField, $list); + $listB = $component->getManipulatedData($gridField, $list); $this->assertEquals( ['Auckland', 'Cologne', 'Melbourne', 'Wellington'], @@ -93,12 +94,13 @@ class GridFieldSortableHeaderTest extends SapphireTest ); // Test one relation 'deep' + $component->setFieldSorting(['Name' => 'Cheerleader.Name']); $state->SortColumn = 'Cheerleader.Name'; $state->SortDirection = 'asc'; - $relationListA = $compontent->getManipulatedData($gridField, $list); + $relationListA = $component->getManipulatedData($gridField, $list); $state->SortDirection = 'desc'; - $relationListB = $compontent->getManipulatedData($gridField, $list); + $relationListB = $component->getManipulatedData($gridField, $list); $this->assertEquals( ['Wellington', 'Melbourne', 'Cologne', 'Auckland'], @@ -110,12 +112,13 @@ class GridFieldSortableHeaderTest extends SapphireTest ); // Test two relations 'deep' + $component->setFieldSorting(['Name' => 'Cheerleader.Hat.Colour']); $state->SortColumn = 'Cheerleader.Hat.Colour'; $state->SortDirection = 'asc'; - $relationListC = $compontent->getManipulatedData($gridField, $list); + $relationListC = $component->getManipulatedData($gridField, $list); $state->SortDirection = 'desc'; - $relationListD = $compontent->getManipulatedData($gridField, $list); + $relationListD = $component->getManipulatedData($gridField, $list); $this->assertEquals( ['Cologne', 'Auckland', 'Wellington', 'Melbourne'], @@ -139,6 +142,7 @@ class GridFieldSortableHeaderTest extends SapphireTest $component = $gridField->getConfig()->getComponentByType(GridFieldSortableHeader::class); // Test that inherited dataobjects will work correctly + $component->setFieldSorting(['Name' => 'Cheerleader.Hat.Colour']); $state->SortColumn = 'Cheerleader.Hat.Colour'; $state->SortDirection = 'asc'; $relationListA = $component->getManipulatedData($gridField, $list); @@ -179,6 +183,7 @@ class GridFieldSortableHeaderTest extends SapphireTest ); // Test subclasses of tables + $component->setFieldSorting(['Name' => 'CheerleadersMom.Hat.Colour']); $state->SortColumn = 'CheerleadersMom.Hat.Colour'; $state->SortDirection = 'asc'; $relationListB = $component->getManipulatedData($gridField, $list); @@ -229,4 +234,21 @@ class GridFieldSortableHeaderTest extends SapphireTest $relationListBdesc->column('City') ); } + + public function testSortColumnValidation() + { + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Invalid SortColumn: INVALID'); + + $list = Team::get()->filter([ 'ClassName' => Team::class ]); + $config = new GridFieldConfig_RecordEditor(); + $gridField = new GridField('testfield', 'testfield', $list, $config); + $component = $gridField->getConfig()->getComponentByType(GridFieldSortableHeader::class); + + $state = $gridField->State->GridFieldSortableHeader; + $state->SortColumn = 'INVALID'; + $state->SortDirection = 'asc'; + + $component->getManipulatedData($gridField, $list); + } }