Skip to content

DQL injection through sorting parameters blocked

Moderate
lchrusciel published GHSA-2xmm-g482-4439 Mar 14, 2022

Package

composer sylius/grid-bundle (Composer)

Affected versions

<1.10.1, 1.11-apha, 1.11-beta, 1.11-rc

Patched versions

1.10.1, 1.11-rc.2

Description

Impact

Values added at the end of query sorting were passed directly to the DB. We don't know, if it could lead to direct SQL injections, however, we should not allow for easy injection of values there anyway.

Patches

The issue is fixed in version 1.10.1 and in 1.11-rc.1

Workarounds

You have to overwrite your Sylius\Component\Grid\Sorting\Sorter.php class:

<?php

// src/App/Sorting/Sorter.php

declare(strict_types=1);

namespace App\Sorting;

use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Sylius\Component\Grid\Data\DataSourceInterface;
use Sylius\Component\Grid\Definition\Grid;
use Sylius\Component\Grid\Parameters;
use Sylius\Component\Grid\Sorting\SorterInterface;

final class Sorter implements SorterInterface
{
    public function sort(DataSourceInterface $dataSource, Grid $grid, Parameters $parameters): void
    {
        $enabledFields = $grid->getFields();
        $expressionBuilder = $dataSource->getExpressionBuilder();

        $sorting = $parameters->get('sorting', $grid->getSorting());
        $this->validateSortingParams($sorting, $enabledFields);

        foreach ($sorting as $field => $order) {
            $this->validateFieldNames($field, $enabledFields);

            $gridField = $grid->getField($field);
            $property = $gridField->getSortable();

            if (null !== $property) {
                $expressionBuilder->addOrderBy($property, $order);
            }
        }
    }

    private function validateSortingParams(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]));
            }
        }
    }

    private function validateFieldNames(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)));
        }
    }
}

and register it in your container:

# config/services.yaml
services:
    # ...
    sylius.grid.sorter:
        class: App\Sorting\Sorter

For more information

If you have any questions or comments about this advisory:

Severity

Moderate

CVE ID

CVE-2022-24752

Weaknesses

Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')

The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data. Learn more on MITRE.

Credits