Make validation execute in stages to avoid possible infinite recursion

This commit is contained in:
tyranron 2022-02-02 18:27:08 +02:00
parent 5304237d7c
commit bb35c66448
No known key found for this signature in database
GPG key ID: 762E144FB230A4F0
6 changed files with 164 additions and 57 deletions

View file

@ -2,6 +2,10 @@
- No changes yet
# [[0.15.9] 2022-02-02](https://github.com/graphql-rust/juniper/releases/tag/juniper-v0.15.9)
- Fix infinite recursion on malformed queries with nested 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.
# [[0.15.8] 2022-01-26](https://github.com/graphql-rust/juniper/releases/tag/juniper-v0.15.8)
- 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.

View file

@ -97,6 +97,10 @@ impl<'a, S: Debug> ValidatorContext<'a, S> {
self.errors.push(RuleError::new(message, locations))
}
pub(crate) fn has_errors(&self) -> bool {
!self.errors.is_empty()
}
#[doc(hidden)]
pub fn into_errors(mut self) -> Vec<RuleError> {
self.errors.sort();

View file

@ -21,6 +21,7 @@ pub use self::{
#[cfg(test)]
pub use self::test_harness::{
expect_fails_rule, expect_fails_rule_with_schema, expect_passes_rule,
expect_fails_fn, expect_fails_fn_with_schema, expect_fails_rule, expect_fails_rule_with_schema,
expect_passes_fn, expect_passes_fn_with_schema, expect_passes_rule,
expect_passes_rule_with_schema,
};

View file

@ -35,7 +35,14 @@ pub fn visit_all_rules<'a, S: Debug>(ctx: &mut ValidatorContext<'a, S>, doc: &'a
where
S: ScalarValue,
{
let mut mv = MultiVisitorNil
// Some validators are depending on the results of other ones.
// For example, validators checking fragments usually rely on the fact that
// they have no cycles (`no_fragment_cycles`), otherwise may stall in an
// infinite recursion. So, we should run validators in stages, moving to the
// next stage only once the previous succeeds. This is better than making
// every single validator being aware of fragments cycles and/or other
// assumptions.
let mut stage1 = MultiVisitorNil
.with(self::arguments_of_correct_type::factory())
.with(self::default_values_of_correct_type::factory())
.with(self::fields_on_correct_type::factory())
@ -49,7 +56,6 @@ where
.with(self::no_undefined_variables::factory())
.with(self::no_unused_fragments::factory())
.with(self::no_unused_variables::factory())
.with(self::overlapping_fields_can_be_merged::factory())
.with(self::possible_fragment_spreads::factory())
.with(self::provided_non_null_arguments::factory())
.with(self::scalar_leafs::factory())
@ -60,6 +66,62 @@ where
.with(self::unique_variable_names::factory())
.with(self::variables_are_input_types::factory())
.with(self::variables_in_allowed_position::factory());
visit(&mut mv, ctx, doc)
visit(&mut stage1, ctx, doc);
if ctx.has_errors() {
return;
}
let mut stage2 = MultiVisitorNil.with(self::overlapping_fields_can_be_merged::factory());
visit(&mut stage2, ctx, doc);
}
#[cfg(test)]
mod tests {
use crate::{parser::SourcePosition, DefaultScalarValue};
use crate::validation::{expect_fails_fn, RuleError};
#[test]
fn handles_recursive_fragments() {
expect_fails_fn::<_, DefaultScalarValue>(
super::visit_all_rules,
"fragment f on QueryRoot { ...f }",
&[
RuleError::new(
"Fragment \"f\" is never used",
&[SourcePosition::new(0, 0, 0)],
),
RuleError::new(
"Cannot spread fragment \"f\"",
&[SourcePosition::new(26, 0, 26)],
),
],
);
}
#[test]
fn handles_nested_recursive_fragments() {
expect_fails_fn::<_, DefaultScalarValue>(
super::visit_all_rules,
"fragment f on QueryRoot { a { ...f a { ...f } } }",
&[
RuleError::new(
"Fragment \"f\" is never used",
&[SourcePosition::new(0, 0, 0)],
),
RuleError::new(
r#"Unknown field "a" on type "QueryRoot""#,
&[SourcePosition::new(26, 0, 26)],
),
RuleError::new(
"Cannot spread fragment \"f\"",
&[SourcePosition::new(30, 0, 30)],
),
RuleError::new(
"Cannot spread fragment \"f\"",
&[SourcePosition::new(39, 0, 39)],
),
],
);
}
}

View file

@ -33,6 +33,7 @@ struct PairSet<'a> {
data: HashMap<&'a str, HashMap<&'a str, bool>>,
}
#[derive(Debug)]
struct OrderedMap<K, V> {
data: HashMap<K, V>,
insert_order: Vec<K>,
@ -172,13 +173,6 @@ impl<'a, S: Debug> OverlappingFieldsCanBeMerged<'a, S> {
);
for frag_name2 in &fragment_names[i + 1..] {
// Prevent infinite fragment recursion. This case is
// caught by fragment validators, but because the validation is
// done in parallel we can't rely on fragments being
// non-recursive here.
if frag_name1 == frag_name2 {
continue;
}
self.collect_conflicts_between_fragments(
&mut conflicts,
frag_name1,
@ -202,10 +196,8 @@ impl<'a, S: Debug> OverlappingFieldsCanBeMerged<'a, S> {
) where
S: ScalarValue,
{
// Prevent infinite fragment recursion. This case is
// caught by fragment validators, but because the validation is
// done in parallel we can't rely on fragments being
// non-recursive here.
// Early return on fragment recursion, as it makes no sense.
// Fragment recursions are prevented by `no_fragment_cycles` validator.
if fragment_name1 == fragment_name2 {
return;
}
@ -293,10 +285,8 @@ impl<'a, S: Debug> OverlappingFieldsCanBeMerged<'a, S> {
self.collect_conflicts_between(conflicts, mutually_exclusive, field_map, &field_map2, ctx);
for fragment_name2 in fragment_names2 {
// Prevent infinite fragment recursion. This case is
// caught by fragment validators, but because the validation is
// done in parallel we can't rely on fragments being
// non-recursive here.
// Early return on fragment recursion, as it makes no sense.
// Fragment recursions are prevented by `no_fragment_cycles` validator.
if fragment_name == fragment_name2 {
return;
}
@ -2279,26 +2269,6 @@ mod tests {
);
}
#[test]
fn handles_recursive_fragments() {
expect_passes_rule_with_schema::<
_,
EmptyMutation<()>,
EmptySubscription<()>,
_,
_,
DefaultScalarValue,
>(
QueryRoot,
EmptyMutation::new(),
EmptySubscription::new(),
factory,
r#"
fragment f on Query { ...f }
"#,
);
}
#[test]
fn error_message_contains_hint_for_alias_conflict() {
assert_eq!(

View file

@ -1,5 +1,5 @@
use crate::{
ast::{FromInputValue, InputValue},
ast::{Document, FromInputValue, InputValue},
executor::Registry,
parser::parse_document_source,
schema::{
@ -812,20 +812,13 @@ where
}
}
pub fn validate<'a, Q, M, Sub, V, F, S>(
r: Q,
m: M,
s: Sub,
q: &'a str,
factory: F,
) -> Vec<RuleError>
pub fn validate<'a, Q, M, Sub, F, S>(r: Q, m: M, s: Sub, q: &'a str, visit_fn: F) -> Vec<RuleError>
where
S: ScalarValue + 'a,
Q: GraphQLType<S, TypeInfo = ()>,
M: GraphQLType<S, TypeInfo = ()>,
Sub: GraphQLType<S, TypeInfo = ()>,
V: Visitor<'a, S> + 'a,
F: Fn() -> V,
F: FnOnce(&mut ValidatorContext<'a, S>, &'a Document<S>),
{
let mut root = RootNode::new_with_scalar_value(r, m, s);
@ -864,10 +857,7 @@ where
parse_document_source(q, &root.schema).expect(&format!("Parse error on input {:#?}", q));
let mut ctx = ValidatorContext::new(unsafe { ::std::mem::transmute(&root.schema) }, &doc);
let mut mv = MultiVisitorNil.with(factory());
visit(&mut mv, &mut ctx, unsafe {
::std::mem::transmute(doc.as_slice())
});
visit_fn(&mut ctx, unsafe { ::std::mem::transmute(doc.as_slice()) });
ctx.into_errors()
}
@ -881,6 +871,14 @@ where
expect_passes_rule_with_schema(QueryRoot, MutationRoot, SubscriptionRoot, factory, q);
}
pub fn expect_passes_fn<'a, F, S>(visit_fn: F, q: &'a str)
where
S: ScalarValue + 'a,
F: FnOnce(&mut ValidatorContext<'a, S>, &'a Document<S>),
{
expect_passes_fn_with_schema(QueryRoot, MutationRoot, SubscriptionRoot, visit_fn, q);
}
pub fn expect_passes_rule_with_schema<'a, Q, M, Sub, V, F, S>(
r: Q,
m: M,
@ -893,9 +891,12 @@ pub fn expect_passes_rule_with_schema<'a, Q, M, Sub, V, F, S>(
M: GraphQLType<S, TypeInfo = ()>,
Sub: GraphQLType<S, TypeInfo = ()>,
V: Visitor<'a, S> + 'a,
F: Fn() -> V,
F: FnOnce() -> V,
{
let errs = validate(r, m, s, q, factory);
let errs = validate(r, m, s, q, move |ctx, doc| {
let mut mv = MultiVisitorNil.with(factory());
visit(&mut mv, ctx, unsafe { ::std::mem::transmute(doc) });
});
if !errs.is_empty() {
print_errors(&errs);
@ -903,6 +904,27 @@ pub fn expect_passes_rule_with_schema<'a, Q, M, Sub, V, F, S>(
}
}
pub fn expect_passes_fn_with_schema<'a, Q, M, Sub, F, S>(
r: Q,
m: M,
s: Sub,
visit_fn: F,
q: &'a str,
) where
S: ScalarValue + 'a,
Q: GraphQLType<S, TypeInfo = ()>,
M: GraphQLType<S, TypeInfo = ()>,
Sub: GraphQLType<S, TypeInfo = ()>,
F: FnOnce(&mut ValidatorContext<'a, S>, &'a Document<S>),
{
let errs = validate(r, m, s, q, visit_fn);
if !errs.is_empty() {
print_errors(&errs);
panic!("Expected `visit_fn` to pass, but errors found");
}
}
pub fn expect_fails_rule<'a, V, F, S>(factory: F, q: &'a str, expected_errors: &[RuleError])
where
S: ScalarValue + 'a,
@ -912,6 +934,14 @@ where
expect_fails_rule_with_schema(QueryRoot, MutationRoot, factory, q, expected_errors);
}
pub fn expect_fails_fn<'a, F, S>(visit_fn: F, q: &'a str, expected_errors: &[RuleError])
where
S: ScalarValue + 'a,
F: FnOnce(&mut ValidatorContext<'a, S>, &'a Document<S>),
{
expect_fails_fn_with_schema(QueryRoot, MutationRoot, visit_fn, q, expected_errors);
}
pub fn expect_fails_rule_with_schema<'a, Q, M, V, F, S>(
r: Q,
m: M,
@ -923,9 +953,18 @@ pub fn expect_fails_rule_with_schema<'a, Q, M, V, F, S>(
Q: GraphQLType<S, TypeInfo = ()>,
M: GraphQLType<S, TypeInfo = ()>,
V: Visitor<'a, S> + 'a,
F: Fn() -> V,
F: FnOnce() -> V,
{
let errs = validate(r, m, crate::EmptySubscription::<S>::new(), q, factory);
let errs = validate(
r,
m,
crate::EmptySubscription::<S>::new(),
q,
move |ctx, doc| {
let mut mv = MultiVisitorNil.with(factory());
visit(&mut mv, ctx, unsafe { ::std::mem::transmute(doc) });
},
);
if errs.is_empty() {
panic!("Expected rule to fail, but no errors were found");
@ -940,6 +979,33 @@ pub fn expect_fails_rule_with_schema<'a, Q, M, V, F, S>(
}
}
pub fn expect_fails_fn_with_schema<'a, Q, M, F, S>(
r: Q,
m: M,
visit_fn: F,
q: &'a str,
expected_errors: &[RuleError],
) where
S: ScalarValue + 'a,
Q: GraphQLType<S, TypeInfo = ()>,
M: GraphQLType<S, TypeInfo = ()>,
F: FnOnce(&mut ValidatorContext<'a, S>, &'a Document<S>),
{
let errs = validate(r, m, crate::EmptySubscription::<S>::new(), q, visit_fn);
if errs.is_empty() {
panic!("Expected `visit_fn`` to fail, but no errors were found");
} else if errs != expected_errors {
println!("==> Expected errors:");
print_errors(expected_errors);
println!("\n==> Actual errors:");
print_errors(&errs);
panic!("Unexpected set of errors found");
}
}
fn print_errors(errs: &[RuleError]) {
for err in errs {
for p in err.locations() {