Security advisory GHSA-2xmm-g482-4439: DQL injection through sorting parameters blocked
Package: sylius/grid-bundle (composer), vulnerable version: v1.10.0, fixed in: 1.10.1
CWE: CWE-89
Files changed by the fix:
src/Component/Sorting/Sorter.php
src/Component/Validation/FieldValidator.php
src/Component/Validation/FieldValidatorInterface.php
src/Component/Validation/SortingParametersValidator.php
src/Component/Validation/SortingParametersValidatorInterface.php

Fix diff:
diff --git a/src/Bundle/Resources/config/services.xml b/src/Bundle/Resources/config/services.xml
index c6a98d17..f22384c9 100644
--- a/src/Bundle/Resources/config/services.xml
+++ b/src/Bundle/Resources/config/services.xml
@@ -46,7 +46,12 @@
             <argument type="service" id="sylius.registry.grid_filter" />
             <argument type="service" id="sylius.grid.filters_criteria_resolver" />
         </service>
-        <service id="sylius.grid.sorter" class="Sylius\Component\Grid\Sorting\Sorter" />
+        <service id="sylius.grid.sorter.validator" class="Sylius\Component\Grid\Validation\SortingParametersValidator" />
+        <service id="sylius.grid.field.validator" class="Sylius\Component\Grid\Validation\FieldValidator" />
+        <service id="sylius.grid.sorter" class="Sylius\Component\Grid\Sorting\Sorter">
+            <argument type="service" id="sylius.grid.sorter.validator" />
+            <argument type="service" id="sylius.grid.field.validator" />
+        </service>
         <service id="sylius.grid.data_source_provider" class="Sylius\Component\Grid\Data\DataSourceProvider">
             <argument type="service" id="sylius.registry.grid_driver" />
         </service>
diff --git a/src/Component/Sorting/Sorter.php b/src/Component/Sorting/Sorter.php
index 139387d0..152312b1 100644
--- a/src/Component/Sorting/Sorter.php
+++ b/src/Component/Sorting/Sorter.php
@@ -13,19 +13,37 @@
 
 namespace Sylius\Component\Grid\Sorting;
 
+use Sylius\Component\Grid\Validation\FieldValidator;
+use Sylius\Component\Grid\Validation\SortingParametersValidator;
 use Sylius\Component\Grid\Data\DataSourceInterface;
 use Sylius\Component\Grid\Definition\Grid;
 use Sylius\Component\Grid\Parameters;
+use Sylius\Component\Grid\Validation\SortingParametersValidatorInterface;
+use Sylius\Component\Grid\Validation\FieldValidatorInterface;
 
 final class Sorter implements SorterInterface
 {
+    private SortingParametersValidatorInterface $sortingValidator;
+
+    private FieldValidatorInterface $fieldValidator;
+
+    public function __construct(?SortingParametersValidatorInterface $sortingValidator = null, ?FieldValidatorInterface $fieldValidator = null)
+    {
+        $this->sortingValidator = $sortingValidator ?? new SortingParametersValidator();
+        $this->fieldValidator = $fieldValidator ?? new FieldValidator();
+    }
+
     public function sort(DataSourceInterface $dataSource, Grid $grid, Parameters $parameters): void
     {
+        $enabledFields = $grid->getEnabledFields();
+
         $expressionBuilder = $dataSource->getExpressionBuilder();
 
         $sorting = $parameters->get('sorting', $grid->getSorting());
+        $this->sortingValidator->validateSortingParameters($sorting, $enabledFields);
 
         foreach ($sorting as $field => $order) {
+            $this->fieldValidator->validateFieldName($field, $enabledFields);
             $gridField = $grid->getField($field);
             $property = $gridField->getSortable();
 
diff --git a/src/Component/Validation/FieldValidator.php b/src/Component/Validation/FieldValidator.php
new file mode 100644
index 00000000..7b89c0ff
--- /dev/null
+++ b/src/Component/Validation/FieldValidator.php
@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Component\Grid\Validation;
+
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+
+final class FieldValidator implements FieldValidatorInterface
+{
+    public function validateFieldName(string $fieldName, array $enabledFields): void
+    {
+        $enabledFieldsNames = array_keys($enabledFields);
+
+        if (!in_array($fieldName, $enabledFieldsNames, true)) {
+            throw new BadRequestHttpException(sprintf('%s is not valid field, did you mean one of these: %s?', $fieldName, implode(', ', $enabledFieldsNames)));
+        }
+    }
+}
diff --git a/src/Component/Validation/FieldValidatorInterface.php b/src/Component/Validation/FieldValidatorInterface.php
new file mode 100644
index 00000000..e76d2f9c
--- /dev/null
+++ b/src/Component/Validation/FieldValidatorInterface.php
@@ -0,0 +1,19 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Component\Grid\Validation;
+
+interface FieldValidatorInterface
+{
+    public function validateFieldName(string $fieldName, array $enabledFields): void;
+}
diff --git a/src/Component/Validation/SortingParametersValidator.php b/src/Component/Validation/SortingParametersValidator.php
new file mode 100644
index 00000000..685c5fe5
--- /dev/null
+++ b/src/Component/Validation/SortingParametersValidator.php
@@ -0,0 +1,28 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Component\Grid\Validation;
+
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+
+final class SortingParametersValidator implements SortingParametersValidatorInterface
+{
+    public function validateSortingParameters(array $sorting, array $enabledFields): void
+    {
+        foreach (array_keys($enabledFields) as $key) {
+            if (array_key_exists($key, $sorting) && !in_array($sorting[$key], ['asc', 'desc'])) {
+                throw new BadRequestHttpException(sprintf('%s is not valid, use asc or desc instead.', $sorting[$key]));
+            }
+        }
+    }
+}
diff --git a/src/Component/Validation/SortingParametersValidatorInterface.php b/src/Component/Validation/SortingParametersValidatorInterface.php
new file mode 100644
index 00000000..ad8bb44b
--- /dev/null
+++ b/src/Component/Validation/SortingParametersValidatorInterface.php
@@ -0,0 +1,19 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Component\Grid\Validation;
+
+interface SortingParametersValidatorInterface
+{
+    public function validateSortingParameters(array $sorting, array $enabledFields): void;
+}
diff --git a/src/Component/composer.json b/src/Component/composer.json
index 25750e49..340df847 100644
--- a/src/Component/composer.json
+++ b/src/Component/composer.json
@@ -33,6 +33,7 @@
         "sylius/registry": "^1.5",
         "symfony/deprecation-contracts": "^2.2",
         "symfony/event-dispatcher": "^4.4 || ^5.2",
+        "symfony/http-kernel": "^4.4 || ^5.2",
         "webmozart/assert": "^1.9"
     },
     "require-dev": {
diff --git a/src/Component/spec/Sorting/SorterSpec.php b/src/Component/spec/Sorting/SorterSpec.php
index b0138183..ef6f953a 100644
--- a/src/Component/spec/Sorting/SorterSpec.php
+++ b/src/Component/spec/Sorting/SorterSpec.php
@@ -20,9 +20,16 @@
 use Sylius\Component\Grid\Definition\Grid;
 use Sylius\Component\Grid\Parameters;
 use Sylius\Component\Grid\Sorting\SorterInterface;
+use Sylius\Component\Grid\Validation\SortingParametersValidatorInterface;
+use Sylius\Component\Grid\Validation\FieldValidatorInterface;
 
 final class SorterSpec extends ObjectBehavior
 {
+    function let(SortingParametersValidatorInterface $sortingValidator, FieldValidatorInterface $fieldValidator): void
+    {
+        $this->beConstructedWith($sortingValidator, $fieldValidator);
+    }
+
     function it_implements_grid_data_source_sorter_interface(): void
     {
         $this->shouldImplement(SorterInterface::class);
@@ -30,58 +37,58 @@ function it_implements_grid_data_source_sorter_interface(): void
 
     function it_sorts_the_data_source_via_expression_builder_based_on_the_grid_definition(
         Grid $grid,
-        Field $nameField,
-        Field $nonSortableField,
+        Field $field,
+        Field $anotherField,
         DataSourceInterface $dataSource,
-        ExpressionBuilderInterface $expressionBuilder
+        ExpressionBuilderInterface $expressionBuilder,
+        SortingParametersValidatorInterface $sortingValidator,
+        FieldValidatorInterface $fieldValidator
     ): void {
         $parameters = new Parameters();
 
         $dataSource->getExpressionBuilder()->willReturn($expressionBuilder);
 
-        $grid->getSorting()->willReturn(['name' => 'desc', 'non_sortable_field' => 'asc']);
+        $grid->getSorting()->willReturn(['name' => 'desc']);
+        $grid->getEnabledFields()->willReturn(['name'=> $field, 'code' => $anotherField]);
 
-        $grid->hasField('name')->willReturn(true);
-        $grid->getField('name')->willReturn($nameField);
-        $nameField->isSortable()->willReturn(true);
-        $nameField->getSortable()->willReturn('translation.name');
+        $sortingValidator->validateSortingParameters(['name' => 'desc'], ['name' => $field , 'code' => $anotherField])->shouldBeCalled();
+        $fieldValidator->validateFieldName('name', ['name' => $field , 'code' => $anotherField])->shouldBeCalled();
 
-        $grid->hasField('non_sortable_field')->willReturn(true);
-        $grid->getField('non_sortable_field')->willReturn($nonSortableField);
-        $nonSortableField->isSortable()->willReturn(false);
-        $nonSortableField->getSortable()->willReturn(null);
+        $grid->hasField('name')->willReturn(true);
+        $grid->getField('name')->willReturn($field);
+        $field->isSortable()->willReturn(true);
+        $field->getSortable()->willReturn('translation.name');
 
         $expressionBuilder->addOrderBy('translation.name', 'desc')->shouldBeCalled();
-        $expressionBuilder->addOrderBy(null, 'asc')->shouldNotBeCalled();
 
         $this->sort($dataSource, $grid, $parameters);
     }
 
     function it_sorts_the_data_source_via_expression_builder_based_on_sorting_parameter(
         Grid $grid,
-        Field $nameField,
-        Field $nonSortableField,
+        Field $field,
+        Field $anotherField,
         DataSourceInterface $dataSource,
-        ExpressionBuilderInterface $expressionBuilder
+        ExpressionBuilderInterface $expressionBuilder,
+        SortingParametersValidatorInterface $sortingValidator,
+        FieldValidatorInterface $fieldValidator
     ): void {
-        $parameters = new Parameters(['sorting' => ['name' => 'asc', 'non_sortable_field' => 'asc']]);
+        $parameters = new Parameters(['sorting' => ['name' => 'asc']]);
 
         $dataSource->getExpressionBuilder()->willReturn($expressionBuilder);
 
-        $grid->getSorting()->willReturn(['code' => 'desc']);
+        $grid->getSorting()->willReturn(['code' => 'asc']);
+        $grid->getEnabledFields()->willReturn(['name' => $field , 'code' => $anotherField]);
 
-        $grid->hasField('name')->willReturn(true);
-        $grid->getField('name')->willReturn($nameField);
-        $nameField->isSortable()->willReturn(true);
-        $nameField->getSortable()->willReturn('translation.name');
+        $sortingValidator->validateSortingParameters(['name' => 'asc'], ['name' => $field , 'code' => $anotherField])->shouldBeCalled();
+        $fieldValidator->validateFieldName('name', ['name' => $field , 'code' => $anotherField])->shouldBeCalled();
 
-        $grid->hasField('non_sortable_field')->willReturn(true);
-        $grid->getField('non_sortable_field')->willReturn($nonSortableField);
-        $nonSortableField->isSortable()->willReturn(false);
-        $nonSortableField->getSortable()->willReturn(null);
+        $grid->hasField('name')->willReturn(true);
+        $grid->getField('name')->willReturn($field);
+        $field->isSortable()->willReturn(true);
+        $field->getSortable()->willReturn('translation.name');
 
         $expressionBuilder->addOrderBy('translation.name', 'asc')->shouldBeCalled();
-        $expressionBuilder->addOrderBy(null, 'asc')->shouldNotBeCalled();
 
         $this->sort($dataSource, $grid, $parameters);
     }
diff --git a/src/Component/spec/Validation/FieldValidatorSpec.php b/src/Component/spec/Validation/FieldValidatorSpec.php
new file mode 100644
index 00000000..efacef0b
--- /dev/null
+++ b/src/Component/spec/Validation/FieldValidatorSpec.php
@@ -0,0 +1,56 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace spec\Sylius\Component\Grid\Validation;
+
+use PhpSpec\ObjectBehavior;
+use Sylius\Component\Grid\Definition\Field;
+use Sylius\Component\Grid\Definition\Grid;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+use Sylius\Component\Grid\Validation\FieldValidatorInterface;
+
+final class FieldValidatorSpec extends ObjectBehavior
+{
+    function it_implements_field_validator_interface(): void
+    {
+        $this->shouldImplement(FieldValidatorInterface::class);
+    }
+
+    function it_throws_exception_if_wrong_field_name_provided(
+        Grid $grid,
+        Field $field,
+        Field $anotherField
+    ): void {
+        $grid->getEnabledFields()->willReturn(['name' => $field , 'code' => $anotherField]);
+        $grid->getSorting()->willReturn(['sorting' => ['non_sortable_field' => 'desc']]);
+
+        $this
+            ->shouldThrow(new BadRequestHttpException('non_sortable_field is not valid field, did you mean one of these: name, code?'))
+            ->during('validateFieldName', ['non_sortable_field', ['name' => $field , 'code' => $anotherField]])
+        ;
+    }
+
+    function it_passes_if_valid_sorting_parameter_provided(
+        Grid $grid,
+        Field $field,
+        Field $anotherField
+    ): void {
+        $grid->getEnabledFields()->willReturn(['name' => $field , 'code' => $anotherField]);
+        $grid->getSorting()->willReturn(['sorting' => ['sortable_field' => 'desc']]);
+
+        $this
+            ->shouldNotThrow(new BadRequestHttpException())
+            ->during('validateFieldName', ['sortable_field', ['sortable_field' => $field , 'code' => $anotherField]])
+        ;
+    }
+}
diff --git a/src/Component/spec/Validation/SortingParametersValidatorSpec.php b/src/Component/spec/Validation/SortingParametersValidatorSpec.php
new file mode 100644
index 00000000..ba19d3d3
--- /dev/null
+++ b/src/Component/spec/Validation/SortingParametersValidatorSpec.php
@@ -0,0 +1,56 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace spec\Sylius\Component\Grid\Validation;
+
+use PhpSpec\ObjectBehavior;
+use Sylius\Component\Grid\Definition\Field;
+use Sylius\Component\Grid\Definition\Grid;
+use Sylius\Component\Grid\Validation\SortingParametersValidatorInterface;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+
+final class SortingParametersValidatorSpec extends ObjectBehavior
+{
+    function it_implements_grid_data_source_sorting_validator_interface(): void
+    {
+        $this->shouldImplement(SortingParametersValidatorInterface::class);
+    }
+
+    function it_throws_exception_if_wrong_sorting_parameter_provided(
+        Grid $grid,
+        Field $field,
+        Field $anotherField
+    ): void {
+        $grid->getEnabledFields()->willReturn(['name' => $field , 'code' => $anotherField]);
+        $grid->getSorting()->willReturn(['name' => 'non_sortable_parameter']);
+
+        $this
+            ->shouldThrow(new BadRequestHttpException('non_sortable_parameter is not valid, use asc or desc instead.'))
+            ->during('validateSortingParameters', [['name' => 'non_sortable_parameter'], ['name' => $field , 'code' => $anotherField]])
+        ;
+    }
+
+    function it_passes_if_valid_sorting_parameter_provided(
+        Grid $grid,
+        Field $field,
+        Field $anotherField
+    ): void {
+        $grid->getEnabledFields()->willReturn(['name' => $field , 'code' => $anotherField]);
+        $grid->getSorting()->willReturn(['name' => 'asc']);
+
+        $this
+            ->shouldNotThrow(new BadRequestHttpException())
+            ->during('validateSortingParameters', [['name' => 'asc'], ['name' => $field , 'code' => $anotherField]])
+        ;
+    }
+}