Skip to content

Commit 6c1d600

Browse files
committed
Address review findings
- Add PHPDoc description for recursionLimit option - Extract DEFAULT_RECURSION_LIMIT constant - Narrow type to int<0, max> to reject negative values statically - Add boundary tests for parseValueLiteral and parseTypeReference - Add CHANGELOG entry for GHSA-r7cg-qjjm-xhqq - Document parser recursion limit in docs/security.md 🤖 Generated with Claude Code
1 parent 7b7f208 commit 6c1d600

5 files changed

Lines changed: 61 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ You can find and compare releases at the [GitHub release page](https://github.co
99

1010
## Unreleased
1111

12+
### Fixed
13+
14+
- Denial of Service via stack overflow from deeply nested queries in the parser https://github.com/webonyx/graphql-php/security/advisories/GHSA-r7cg-qjjm-xhqq
15+
1216
## v15.32.2
1317

1418
### Fixed

docs/class-reference.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1158,7 +1158,7 @@ Parses string containing GraphQL query language or [schema definition language](
11581158
allowLegacySDLEmptyFields?: bool,
11591159
allowLegacySDLImplementsInterfaces?: bool,
11601160
experimentalFragmentVariables?: bool,
1161-
recursionLimit?: int
1161+
recursionLimit?: int<0, max>
11621162
}
11631163
```
11641164

@@ -1189,6 +1189,11 @@ Parses string containing GraphQL query language or [schema definition language](
11891189

11901190
Note: this feature is experimental and may change or be removed in the future.
11911191

1192+
- **recursionLimit**:
1193+
Limits the depth of recursion during parsing to prevent stack overflows from deeply nested queries.
1194+
The counter is shared across `parseSelectionSet`, `parseValueLiteral`, and `parseTypeReference`.
1195+
Defaults to 256. Set to 0 to disable the limit.
1196+
11921197
Those magic functions allow partial parsing:
11931198

11941199
@method static NameNode name(Source|string $source, ParserOptions $options = [])

docs/security.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,29 @@ At a basic level, it is recommended to limit the resources a single HTTP request
1111

1212
In addition, graphql-php offers security mechanisms that are specific to GraphQL.
1313

14+
## Parser Recursion Limit
15+
16+
The parser uses recursive descent to handle nested structures such as selection sets, list values, and list types.
17+
Without a depth limit, a deeply nested query can cause a PHP stack overflow (SIGSEGV) that cannot be caught.
18+
19+
By default, the parser limits recursion depth to **256**.
20+
This is well above realistic queries (typically 10–20 levels deep) and far below the threshold that would crash PHP.
21+
22+
You can customize the limit through the `recursionLimit` parser option:
23+
24+
```php
25+
use GraphQL\Language\Parser;
26+
27+
// Custom limit
28+
$ast = Parser::parse($source, ['recursionLimit' => 128]);
29+
30+
// Disable the limit (not recommended for untrusted input)
31+
$ast = Parser::parse($source, ['recursionLimit' => 0]);
32+
```
33+
34+
The limit is shared across all recursive entry points (`parseSelectionSet`, `parseValueLiteral`, `parseTypeReference`).
35+
When exceeded, the parser throws a `SyntaxError` before the PHP stack overflows.
36+
1437
## Query Complexity Analysis
1538

1639
This is a port of [Query Complexity Analysis in Sangria](https://sangria-graphql.github.io/learn#query-complexity-analysis).

src/Language/Parser.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
* allowLegacySDLEmptyFields?: bool,
6767
* allowLegacySDLImplementsInterfaces?: bool,
6868
* experimentalFragmentVariables?: bool,
69-
* recursionLimit?: int
69+
* recursionLimit?: int<0, max>
7070
* }
7171
*
7272
* - **noLocation**:
@@ -96,6 +96,11 @@
9696
*
9797
* Note: this feature is experimental and may change or be removed in the future.
9898
*
99+
* - **recursionLimit**:
100+
* Limits the depth of recursion during parsing to prevent stack overflows from deeply nested queries.
101+
* The counter is shared across `parseSelectionSet`, `parseValueLiteral`, and `parseTypeReference`.
102+
* Defaults to 256. Set to 0 to disable the limit.
103+
*
99104
* Those magic functions allow partial parsing:
100105
*
101106
* @method static NameNode name(Source|string $source, ParserOptions $options = [])
@@ -168,6 +173,9 @@
168173
*/
169174
class Parser
170175
{
176+
/** @api */
177+
public const DEFAULT_RECURSION_LIMIT = 256;
178+
171179
/**
172180
* Given a GraphQL source, parses it into a `GraphQL\Language\AST\DocumentNode`.
173181
*
@@ -333,7 +341,7 @@ public function __construct($source, array $options = [])
333341
? $source
334342
: new Source($source);
335343
$this->lexer = new Lexer($sourceObj, $options);
336-
$this->recursionLimit = $options['recursionLimit'] ?? 256;
344+
$this->recursionLimit = $options['recursionLimit'] ?? self::DEFAULT_RECURSION_LIMIT;
337345
}
338346

339347
/**

tests/Language/ParserTest.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,24 @@ public function testQueryAtExactLimitPasses(): void
782782
$this->expectNotToPerformAssertions();
783783
}
784784

785+
public function testValueLiteralAtExactLimitPasses(): void
786+
{
787+
$depth = 5;
788+
$query = '{ field(arg: ' . str_repeat('[', $depth) . '1' . str_repeat(']', $depth) . ') }';
789+
790+
Parser::parse($query, ['recursionLimit' => $depth + 2]);
791+
$this->expectNotToPerformAssertions();
792+
}
793+
794+
public function testTypeReferenceAtExactLimitPasses(): void
795+
{
796+
$depth = 5;
797+
$query = 'query ($var: ' . str_repeat('[', $depth) . 'Int' . str_repeat(']', $depth) . ') { field }';
798+
799+
Parser::parse($query, ['recursionLimit' => $depth + 2]);
800+
$this->expectNotToPerformAssertions();
801+
}
802+
785803
public function testSiblingBranchesDontAccumulate(): void
786804
{
787805
$limit = 5;

0 commit comments

Comments
 (0)