This issue has been reported in private to the Drupal Security Team, but it has been decided to fix this in public without a security release as there is no direct vulernability.
Reported by mingsong:
You can see this vulnerability by:
1. Enabling the module
2. Create a GraphQL server that uses the EntityQuery data producer (Drupal\graphql\Plugin\GraphQL\DataProducer\Entity\EntityQuery).
3. Now an entity query is exposed by the GraphQL server, in which could accept a query operator as a query parameter via a GraphQL request.
The problem comes from the code in line 135 on /graphql/src/Plugin/GraphQL/DataProducer/Entity/EntityQueryBase.php
(https://git.drupalcode.org/project/graphql/-/blob/8.x-4.x/src/Plugin/Gra...)
which accepts a user input parameter as the operator.
For example,
$registry->addFieldResolver('Query', 'jobApplicationsByUserId',
$builder->compose(
$builder->fromArgument('id'),
$builder->callback(function ($uid) {
$conditions = [
[
'field' => 'uid',
'value' => [$uid],
'operator' => $builder->fromArgument('operator'),
],
];
return $conditions;
}),
$builder->produce('entity_query', [
'type' => $builder->fromValue('node'),
'conditions' => $builder->fromParent(),
]),
$builder->produce('entity_load_multiple', [
'type' => $builder->fromValue('node'),
'ids' => $builder->fromParent(),
'conditions' => $builder->fromArgument('operator'),
]),
)
);
When an injected operator string bypass the checks by Drupal API, such as 'IS NOT NULL OR 0<>', it might bypass the access check too, then return unexpected result to user via GraphQL.
According to the guideline of 'Writing secure code for Drupal' ( https://www.drupal.org/docs/administering-a-drupal-site/security-in-drup... → ), this is not a good practice which might expose a SQL injection vulnerability.
The developer made the mistake to pass user provided data as operator.
We can implement an allow list for the operator to hard-code secure operators. This will be a minor compatibility break - theoretically. I cannot imagine that a developer would want to have extended SQL syntax in the operator, so it is fine to break such use cases. We can explain that in future release notes.
That way the entity_query data producer is secure by default and developers cannot make mistakes as easily.
Implement changes.
The operator for entity_query will reject complex expressions.
Active
4.0
Code
It is used for security vulnerabilities which do not need a security advisory. For example, security issues in projects which do not have security advisory coverage, or forward-porting a change already disclosed in a security advisory. See Drupal’s security advisory policy for details. Be careful publicly disclosing security vulnerabilities! Use the “Report a security vulnerability” link in the project page’s sidebar. See how to report a security issue for details.
Thanks for summarizing the conversation so far.
I think this is the right tag to use these days.
Merge request created. @kingdutch can you review?