Skip to content

Commit bd41cf7

Browse files
committed
fix: deduplicate fields in OverlappingFieldsCanBeMerged to prevent DoS
The validation rule had O(n²) time complexity when processing queries with many repeated fields sharing the same response name. A query like `{ hello hello hello ... }` with thousands of fields caused excessive CPU usage, exploitable as a denial of service vector. Fields that are structurally identical (same parent type, field name, arguments, directives, and selection set) can never conflict with each other, so we deduplicate them before pairwise comparison. This ports the graphql-php fix for GHSA-68jq-c3rv-pcrr: webonyx/graphql-php@b5e7c99 Assisted-by: Claude:claude-fable-5
1 parent 56d4338 commit bd41cf7

3 files changed

Lines changed: 143 additions & 3 deletions

File tree

src/graphql/validation/rules/overlapping_fields_can_be_merged.py

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,8 +539,11 @@ def collect_conflicts_within(
539539
# (except to itself). If the list only has one item, nothing needs to be
540540
# compared.
541541
if len(fields) > 1:
542-
for i, field in enumerate(fields):
543-
for other_field in fields[i + 1 :]:
542+
# Deduplicate structurally identical fields to avoid quadratic blowup
543+
# when a query repeats the same field many times.
544+
unique_fields = deduplicate_fields(fields)
545+
for i, field in enumerate(unique_fields):
546+
for other_field in unique_fields[i + 1 :]:
544547
conflict = find_conflict(
545548
context,
546549
cached_fields_and_fragment_spreads,
@@ -558,6 +561,51 @@ def collect_conflicts_within(
558561
conflicts.append(conflict)
559562

560563

564+
def deduplicate_fields(fields: list[NodeAndDef]) -> list[NodeAndDef]:
565+
"""Deduplicate structurally identical fields.
566+
567+
Fields that are structurally identical (same parent type, field name, arguments,
568+
directives and selection set) can never conflict with each other, so only one of
569+
them needs to take part in the pairwise conflict comparison.
570+
"""
571+
unique: list[NodeAndDef] = []
572+
seen: set[str] = set()
573+
for field in fields:
574+
key = field_fingerprint(field)
575+
if key not in seen:
576+
seen.add(key)
577+
unique.append(field)
578+
return unique
579+
580+
581+
def field_fingerprint(field: NodeAndDef) -> str:
582+
"""Build a fingerprint identifying the structure of a given field.
583+
584+
Two fields that share a fingerprint are structurally identical and therefore
585+
cannot conflict with each other, so only one needs to take part in the pairwise
586+
conflict comparison. The fingerprint is derived from normalized AST content
587+
rather than node identity, so that repeated occurrences of the same composite
588+
field (each of which has a distinct ``SelectionSetNode`` instance) collapse. It
589+
mirrors the equivalence relation used by ``find_conflict``: arguments are
590+
compared order-independently with normalized values (see ``same_arguments``),
591+
and directives are included in a stable order because differing stream
592+
directives are treated as a conflict (see ``same_streams``).
593+
"""
594+
parent_type, node, _ = field
595+
parts: list[str] = [parent_type.name if parent_type else "", node.name.value]
596+
parts.extend(
597+
f"{argument.name.value}={stringify_value(argument.value)}"
598+
for argument in sorted(node.arguments or (), key=lambda arg: arg.name.value)
599+
)
600+
parts.extend(
601+
print_ast(directive)
602+
for directive in sorted(node.directives or (), key=print_ast)
603+
)
604+
if node.selection_set:
605+
parts.append(print_ast(node.selection_set))
606+
return "\x00".join(parts)
607+
608+
561609
def collect_conflicts_between(
562610
context: ValidationContext,
563611
conflicts: list[Conflict],
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import pytest
2+
3+
from graphql import (
4+
GraphQLField,
5+
GraphQLObjectType,
6+
GraphQLSchema,
7+
GraphQLString,
8+
parse,
9+
validate,
10+
)
11+
12+
schema = GraphQLSchema(
13+
query=GraphQLObjectType(
14+
name="Query",
15+
fields={"hello": GraphQLField(GraphQLString)},
16+
)
17+
)
18+
19+
20+
@pytest.mark.parametrize("count", [100, 500, 1000, 2000, 3000])
21+
def test_validate_many_repeated_fields(benchmark, count):
22+
document = parse(f"{{ {'hello ' * count}}}")
23+
result = benchmark(lambda: validate(schema, document))
24+
assert result == []

tests/validation/test_overlapping_fields_can_be_merged.py

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
from functools import partial
22

3+
import pytest
4+
5+
from graphql import parse, validate
36
from graphql.utilities import build_schema
47
from graphql.validation import OverlappingFieldsCanBeMergedRule
58

6-
from .harness import assert_validation_errors
9+
from .harness import assert_validation_errors, test_schema
710

811
assert_errors = partial(assert_validation_errors, OverlappingFieldsCanBeMergedRule)
912

@@ -1565,6 +1568,71 @@ def does_not_infinite_loop_on_transitively_recursive_fragments():
15651568
"""
15661569
)
15671570

1571+
def many_repeated_fields_do_not_cause_quadratic_blowup():
1572+
repeated_fields = "name " * 3000
1573+
assert_valid(
1574+
f"""
1575+
fragment manyRepeatedFields on Dog {{
1576+
{repeated_fields}
1577+
}}
1578+
"""
1579+
)
1580+
1581+
def many_repeated_fields_with_conflict_still_detected():
1582+
repeated_fields = "name " * 100
1583+
doc = parse(
1584+
f"""
1585+
fragment conflictsAmongMany on Dog {{
1586+
{repeated_fields}
1587+
name: nickname
1588+
}}
1589+
"""
1590+
)
1591+
errors = validate(test_schema, doc, [OverlappingFieldsCanBeMergedRule])
1592+
assert errors
1593+
assert "'name' and 'nickname' are different fields" in errors[0].message
1594+
1595+
@pytest.mark.timeout(5)
1596+
def many_repeated_composite_fields_do_not_cause_quadratic_blowup():
1597+
# Each occurrence of a composite field has its own SelectionSetNode, so
1598+
# deduplication must fingerprint the selection set by content, not identity,
1599+
# otherwise these still trigger quadratic behavior.
1600+
repeated_fields = "mother { name } " * 3000
1601+
assert_valid(
1602+
f"""
1603+
fragment manyRepeatedCompositeFields on Dog {{
1604+
{repeated_fields}
1605+
}}
1606+
"""
1607+
)
1608+
1609+
@pytest.mark.timeout(5)
1610+
def many_repeated_fields_with_reordered_arguments_do_not_cause_quadratic_blowup():
1611+
# Fields whose arguments differ only in order are equivalent and must
1612+
# deduplicate, so the fingerprint has to be argument-order independent.
1613+
repeated_fields = "isAtLocation(x: 1, y: 2) isAtLocation(y: 2, x: 1) " * 1500
1614+
assert_valid(
1615+
f"""
1616+
fragment reorderedArgs on Dog {{
1617+
{repeated_fields}
1618+
}}
1619+
"""
1620+
)
1621+
1622+
def many_repeated_composite_fields_with_conflict_still_detected():
1623+
repeated_fields = "mother { name } " * 100
1624+
doc = parse(
1625+
f"""
1626+
fragment conflictsAmongManyComposites on Dog {{
1627+
{repeated_fields}
1628+
mother {{ name: nickname }}
1629+
}}
1630+
"""
1631+
)
1632+
errors = validate(test_schema, doc, [OverlappingFieldsCanBeMergedRule])
1633+
assert errors
1634+
assert "'name' and 'nickname' are different fields" in errors[0].message
1635+
15681636
def finds_invalid_case_even_with_immediately_recursive_fragment():
15691637
assert_errors(
15701638
"""

0 commit comments

Comments
 (0)