Skip to content

Commit 17d474e

Browse files
committed
Fix panic on malformed queries with recursive fragments.
This is a potential denial-of-service attack vector. Thanks to [@quapka](https://github.com/quapka) for the detailed vulnerability report and reproduction steps.
1 parent 1aa1000 commit 17d474e

File tree

2 files changed

+42
-0
lines changed

2 files changed

+42
-0
lines changed

juniper/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# master
22

3+
## Security
4+
5+
- Fix panic on malformed queries with recursive fragments. *This is a potential denial-of-service attack vector.* Thanks to [@quapka](https://github.com/quapka) for the detailed vulnerability report and reproduction steps.
6+
37
## Breaking Changes
48

59
- Replaced `Visitor` associated type with `DeserializeOwned` requirement in `ScalarValue` trait. ([#985](https://github.com/graphql-rust/juniper/pull/985))

juniper/src/validation/rules/overlapping_fields_can_be_merged.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,13 @@ impl<'a, S: Debug> OverlappingFieldsCanBeMerged<'a, S> {
172172
);
173173

174174
for frag_name2 in &fragment_names[i + 1..] {
175+
// Prevent infinite fragment recursion. This case is
176+
// caught by fragment validators, but because the validation is
177+
// done in parallel we can't rely on fragments being
178+
// non-recursive here.
179+
if frag_name1 == frag_name2 {
180+
continue;
181+
}
175182
self.collect_conflicts_between_fragments(
176183
&mut conflicts,
177184
frag_name1,
@@ -195,6 +202,10 @@ impl<'a, S: Debug> OverlappingFieldsCanBeMerged<'a, S> {
195202
) where
196203
S: ScalarValue,
197204
{
205+
// Prevent infinite fragment recursion. This case is
206+
// caught by fragment validators, but because the validation is
207+
// done in parallel we can't rely on fragments being
208+
// non-recursive here.
198209
if fragment_name1 == fragment_name2 {
199210
return;
200211
}
@@ -282,6 +293,13 @@ impl<'a, S: Debug> OverlappingFieldsCanBeMerged<'a, S> {
282293
self.collect_conflicts_between(conflicts, mutually_exclusive, field_map, &field_map2, ctx);
283294

284295
for fragment_name2 in fragment_names2 {
296+
// Prevent infinite fragment recursion. This case is
297+
// caught by fragment validators, but because the validation is
298+
// done in parallel we can't rely on fragments being
299+
// non-recursive here.
300+
if fragment_name == fragment_name2 {
301+
return;
302+
}
285303
self.collect_conflicts_between_fields_and_fragment(
286304
conflicts,
287305
field_map,
@@ -2267,6 +2285,26 @@ mod tests {
22672285
);
22682286
}
22692287

2288+
#[test]
2289+
fn handles_recursive_fragments() {
2290+
expect_passes_rule_with_schema::<
2291+
_,
2292+
EmptyMutation<()>,
2293+
EmptySubscription<()>,
2294+
_,
2295+
_,
2296+
DefaultScalarValue,
2297+
>(
2298+
QueryRoot,
2299+
EmptyMutation::new(),
2300+
EmptySubscription::new(),
2301+
factory,
2302+
r#"
2303+
fragment f on Query { ...f }
2304+
"#,
2305+
);
2306+
}
2307+
22702308
#[test]
22712309
fn error_message_contains_hint_for_alias_conflict() {
22722310
assert_eq!(

0 commit comments

Comments
 (0)