Skip to content
Permalink
Browse files Browse the repository at this point in the history
bug #222 [Security] Dql injection through sorting parameters blocked …
…(TheMilek)

This PR was merged into the 1.10 branch.

Discussion
----------



Commits
-------

b702009 Dql injection through sorting parameters blocked
  • Loading branch information
lchrusciel committed Mar 14, 2022
2 parents 789ab3a + b702009 commit 73d0791
Show file tree
Hide file tree
Showing 10 changed files with 265 additions and 28 deletions.
7 changes: 6 additions & 1 deletion src/Bundle/Resources/config/services.xml
Expand Up @@ -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>
Expand Down
18 changes: 18 additions & 0 deletions src/Component/Sorting/Sorter.php
Expand Up @@ -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();

Expand Down
28 changes: 28 additions & 0 deletions 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)));
}
}
}
19 changes: 19 additions & 0 deletions 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;
}
28 changes: 28 additions & 0 deletions 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]));
}
}
}
}
19 changes: 19 additions & 0 deletions 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;
}
1 change: 1 addition & 0 deletions src/Component/composer.json
Expand Up @@ -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": {
Expand Down
61 changes: 34 additions & 27 deletions src/Component/spec/Sorting/SorterSpec.php
Expand Up @@ -20,68 +20,75 @@
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);
}

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);
}
Expand Down
56 changes: 56 additions & 0 deletions 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]])
;
}
}
56 changes: 56 additions & 0 deletions 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]])
;
}
}

0 comments on commit 73d0791

Please sign in to comment.