Skip to content

Commit 7862670

Browse files
committed
Mitigate DoS risk from quadratic query validation complexity.
Upgrades webonyx/graphql-php to ^15.31.5 and adapts the module to graphql-php v15 (QueryValidationContext, ValidationRule::getVisitor, persisted query loader naming, OperationParams property access). Updates PHPUnit tests for current PHPUnit APIs (onlyMethods/addMethods, static data providers). Addresses the security concern: Potential Denial of Service via quadratic complexity in drupal/graphql query validation. Credit: David Gallego (gallegodavid) for the fix.
1 parent 8c74012 commit 7862670

7 files changed

Lines changed: 19 additions & 19 deletions

File tree

composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"homepage": "http://drupal.org/project/graphql",
66
"license": "GPL-2.0+",
77
"require": {
8-
"webonyx/graphql-php": "^14.11.8"
8+
"webonyx/graphql-php": "^15.31.5"
99
},
1010
"minimum-stability": "dev"
1111
}

src/Access/QueryAccessCheck.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public function access(AccountInterface $account) {
5252
// If a query was provided by the user, this is an arbitrary query (it's
5353
// not a persisted query). Hence, we only grant access if the user has the
5454
// permission to execute any query.
55-
if ($operation->getOriginalInput('query')) {
55+
if (isset($operation->originalInput['query'])) {
5656
return AccessResult::allowedIfHasPermission($account, 'execute graphql requests');
5757
}
5858
}

src/GraphQL/Execution/QueryProcessor.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
use GraphQL\Utils\AST;
2525
use GraphQL\Utils\TypeInfo;
2626
use GraphQL\Utils\Utils;
27-
use GraphQL\Validator\Rules\AbstractValidationRule;
27+
use GraphQL\Validator\Rules\ValidationRule;
2828
use GraphQL\Validator\Rules\QueryComplexity;
29-
use GraphQL\Validator\ValidationContext;
29+
use GraphQL\Validator\QueryValidationContext;
3030
use Symfony\Component\HttpFoundation\RequestStack;
3131

3232
/**
@@ -197,7 +197,7 @@ protected function executeOperation(PromiseAdapter $adapter, ServerConfig $confi
197197
// in the same document, hence this check is sufficient.
198198
$operation = $params->operation;
199199
$type = AST::getOperationAST($document, $operation);
200-
if ($params->isReadOnly() && $type->operation !== 'query') {
200+
if ($params->readOnly && $type->operation !== 'query') {
201201
throw new RequestError('GET requests are only supported for query operations.');
202202
}
203203

@@ -352,14 +352,14 @@ protected function validateOperation(ServerConfig $config, OperationParams $para
352352

353353
$schema = $config->getSchema();
354354
$info = new TypeInfo($schema);
355-
$validation = new ValidationContext($schema, $document, $info);
356-
$visitors = array_values(array_map(function (AbstractValidationRule $rule) use ($validation, $params) {
355+
$validation = new QueryValidationContext($schema, $document, $info);
356+
$visitors = array_values(array_map(function (ValidationRule $rule) use ($validation, $params) {
357357
// Set current variable values for QueryComplexity validation rule case.
358358
// @see \GraphQL\GraphQL::promiseToExecute for equivalent
359359
if ($rule instanceof QueryComplexity && !empty($params->variables)) {
360360
$rule->setRawVariableValues($params->variables);
361361
}
362-
return $rule($validation);
362+
return $rule->getVisitor($validation);
363363
}, $rules));
364364

365365
// Run the query visitor with the prepared validation rules and the cache
@@ -434,7 +434,7 @@ protected function resolveValidationRules(ServerConfig $config, OperationParams
434434
* @throws \GraphQL\Server\RequestError
435435
*/
436436
protected function loadPersistedQuery(ServerConfig $config, OperationParams $params) {
437-
if (!$loader = $config->getPersistentQueryLoader()) {
437+
if (!$loader = $config->getPersistedQueryLoader()) {
438438
throw new RequestError('Persisted queries are not supported by this server.');
439439
}
440440

src/Plugin/GraphQL/Schemas/SchemaPluginBase.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ public function getServer() {
277277
});
278278

279279
$config->setValidationRules(function (OperationParams $params, DocumentNode $document, $operation) {
280-
if (isset($params->queryId) && empty($params->getOriginalInput('query'))) {
280+
if (isset($params->queryId) && empty($params->originalInput['query'])) {
281281
// Assume that pre-parsed documents are already validated. This allows
282282
// us to store pre-validated query documents e.g. for persisted queries
283283
// effectively improving performance by skipping run-time validation.
@@ -287,7 +287,7 @@ public function getServer() {
287287
return array_values(DocumentValidator::defaultRules());
288288
});
289289

290-
$config->setPersistentQueryLoader([$this->queryProvider, 'getQuery']);
290+
$config->setPersistedQueryLoader([$this->queryProvider, 'getQuery']);
291291
$config->setQueryBatching(TRUE);
292292
$config->setDebugFlag($this->parameters['development'] ? DebugFlag::INCLUDE_DEBUG_MESSAGE : DebugFlag::NONE);
293293
$config->setSchema($this->getSchema());

tests/src/Kernel/Framework/BufferedFieldTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class BufferedFieldTest extends GraphQLTestBase {
1717
*/
1818
public function testBatchedFields() {
1919
$buffer = $this->getMockBuilder(BufferBase::class)
20-
->setMethods(['resolveBufferArray'])
20+
->onlyMethods(['resolveBufferArray'])
2121
->getMock();
2222

2323
$users = [

tests/src/Traits/MockGraphQLPluginTrait.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,14 @@ protected function injectTypeSystemPluginManagers(ContainerBuilder $container) {
124124

125125
$mockFactory = $this
126126
->getMockBuilder(FactoryInterface::class)
127-
->setMethods([
127+
->onlyMethods([
128128
'createInstance',
129129
])
130130
->getMock();
131131

132132
$mockDiscovery = $this
133133
->getMockBuilder(DiscoveryInterface::class)
134-
->setMethods([
134+
->onlyMethods([
135135
'hasDefinition',
136136
'getDefinitions',
137137
'getDefinition',
@@ -343,7 +343,7 @@ protected function mockField($id, $definition, $result = NULL, $builder = NULL)
343343
protected function mockFieldFactory($definition, $result = NULL, $builder = NULL) {
344344
$field = $this->getMockBuilder(FieldPluginBase::class)
345345
->setConstructorArgs([[], $definition['id'], $definition])
346-
->setMethods([
346+
->onlyMethods([
347347
'resolveValues',
348348
])->getMock();
349349

@@ -396,7 +396,7 @@ protected function mockType($id, array $definition, $applies = TRUE, $builder =
396396
protected function mockTypeFactory($definition, $applies = TRUE, $builder = NULL) {
397397
$type = $this->getMockBuilder(TypePluginBase::class)
398398
->setConstructorArgs([[], $definition['id'], $definition])
399-
->setMethods([
399+
->onlyMethods([
400400
'applies',
401401
])->getMock();
402402

@@ -486,7 +486,7 @@ protected function mockMutation($id, array $definition, $result = NULL, $builder
486486
protected function mockMutationFactory($definition, $result = NULL, $builder = NULL) {
487487
$mutation = $this->getMockBuilder(MutationPluginBase::class)
488488
->setConstructorArgs([[], $definition['id'], $definition])
489-
->setMethods([
489+
->addMethods([
490490
'resolve',
491491
])->getMock();
492492

@@ -618,7 +618,7 @@ protected function mockEnum($id, array $definition, $values = [], $builder = NUL
618618
protected function mockEnumFactory($definition, $values = [], $builder = NULL) {
619619
$enum = $this->getMockBuilder(EnumPluginBase::class)
620620
->setConstructorArgs([[], $definition['id'], $definition])
621-
->setMethods([
621+
->onlyMethods([
622622
'buildEnumValues',
623623
])->getMock();
624624

tests/src/Unit/StringFormattingTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public function testPropCaseFormatting($input, $expected) {
4444
/**
4545
*
4646
*/
47-
public function providerTestStringFormatting() {
47+
public static function providerTestStringFormatting() {
4848
return [
4949
[['simple-name'], 'SimpleName'],
5050
[['123-name-with*^&!@some-SPECIAL-chars'], '_123NameWithSomeSPECIALChars'],

0 commit comments

Comments
 (0)