Skip to content

Commit 8bfd4c4

Browse files
authored
Add field merging budget (#9520)
1 parent e189d95 commit 8bfd4c4

34 files changed

Lines changed: 1007 additions & 81 deletions

src/HotChocolate/Core/benchmarks/Validation.Benchmarks/OverlappingFieldsMergedBenchmark.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ public void FragmentHeavyQuery()
149149
private DocumentValidatorContext CreateContext(DocumentNode document)
150150
{
151151
var context = new DocumentValidatorContext();
152-
context.Initialize(_schema, default, document, 100, 100, null);
152+
context.Initialize(_schema, default, document, 100, 100, 1_000, null);
153153
return context;
154154
}
155155

src/HotChocolate/Core/src/Types.Analyzers/Errors.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public static class Errors
5252

5353
public static readonly DiagnosticDescriptor InterfaceTypeStaticKeywordMissing =
5454
new(
55-
id: "HC0107",
55+
id: "HC0112",
5656
title: "Static Keyword Missing",
5757
messageFormat: "A split interface type class needs to be a static class",
5858
category: "TypeSystem",
@@ -79,7 +79,7 @@ public static class Errors
7979

8080
public static readonly DiagnosticDescriptor DataLoaderCannotBeGeneric =
8181
new(
82-
id: "HC0085",
82+
id: "HC0111",
8383
title: "DataLoader Cannot Be Generic",
8484
messageFormat: "The DataLoader source generator cannot generate generic DataLoaders",
8585
category: "DataLoader",
@@ -88,7 +88,7 @@ public static class Errors
8888

8989
public static readonly DiagnosticDescriptor ConnectionSingleGenericTypeArgument =
9090
new(
91-
id: "HC0086",
91+
id: "HC0110",
9292
title: "Invalid Connection Structure",
9393
messageFormat: "A generic connection/edge type must have a single generic type argument that represents the node type",
9494
category: "TypeSystem",
@@ -97,7 +97,7 @@ public static class Errors
9797

9898
public static readonly DiagnosticDescriptor ConnectionNameFormatIsInvalid =
9999
new(
100-
id: "HC0087",
100+
id: "HC0109",
101101
title: "Invalid Connection/Edge Name Format",
102102
messageFormat: "A connection/edge name must be in the format `{0}Edge` or `{0}Connection`",
103103
category: "TypeSystem",
@@ -106,7 +106,7 @@ public static class Errors
106106

107107
public static readonly DiagnosticDescriptor ConnectionNameDuplicate =
108108
new(
109-
id: "HC0088",
109+
id: "HC0108",
110110
title: "Invalid Connection/Edge Name",
111111
messageFormat: "The type `{0}` cannot be mapped to the GraphQL type name `{1}` as `{2}` is already mapped to it",
112112
category: "TypeSystem",

src/HotChocolate/Core/src/Types/Execution/DependencyInjection/RequestExecutorBuilderExtensions.Validation.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,36 @@ public static IRequestExecutorBuilder RemoveMaxAllowedFieldCycleDepthRule(
328328
return builder;
329329
}
330330

331+
/// <summary>
332+
/// Sets the maximum allowed field merge comparisons during
333+
/// overlapping-fields-can-be-merged validation.
334+
/// </summary>
335+
/// <param name="builder">
336+
/// The <see cref="IRequestExecutorBuilder"/>.
337+
/// </param>
338+
/// <param name="maxAllowedFieldMergeComparisons">
339+
/// The maximum number of field-merge comparisons.
340+
/// </param>
341+
/// <returns>
342+
/// Returns an <see cref="IRequestExecutorBuilder"/> that can be used to chain
343+
/// configuration.
344+
/// </returns>
345+
/// <exception cref="ArgumentNullException">
346+
/// <paramref name="builder"/> is <c>null</c>.
347+
/// </exception>
348+
public static IRequestExecutorBuilder SetMaxAllowedFieldMergeComparisons(
349+
this IRequestExecutorBuilder builder,
350+
int maxAllowedFieldMergeComparisons)
351+
{
352+
ArgumentNullException.ThrowIfNull(builder);
353+
354+
ConfigureValidation(
355+
builder,
356+
(_, b) => b.ModifyOptions(o => o.MaxAllowedFieldMergeComparisons = maxAllowedFieldMergeComparisons));
357+
358+
return builder;
359+
}
360+
331361
/// <summary>
332362
/// Configures the underlying <see cref="DocumentValidatorBuilder"/>.
333363
/// </summary>

src/HotChocolate/Core/src/Types/Execution/DependencyInjection/RequestExecutorServiceCollectionExtensions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ public static IServiceCollection AddGraphQLCore(this IServiceCollection services
8585
maxAllowedNodes: options.MaxAllowedNodes,
8686
maxAllowedTokens: options.MaxAllowedTokens,
8787
maxAllowedFields: options.MaxAllowedFields,
88+
maxAllowedDirectives: options.MaxAllowedDirectives,
8889
maxAllowedRecursionDepth: options.MaxAllowedRecursionDepth);
8990
});
9091

src/HotChocolate/Core/src/Types/Execution/Options/RequestParserOptions.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,15 @@ public sealed class RequestParserOptions
5555
/// </summary>
5656
public int MaxAllowedFields { get; set; } = 2048;
5757

58+
/// <summary>
59+
/// <para>
60+
/// The maximum number of directives allowed per location (e.g. per field,
61+
/// per operation, per fragment definition). Repeatable directives can be used
62+
/// to exhaust CPU and memory resources if not limited.
63+
/// </para>
64+
/// </summary>
65+
public int MaxAllowedDirectives { get; set; } = 4;
66+
5867
/// <summary>
5968
/// <para>
6069
/// The maximum allowed recursion depth when parsing a document.

src/HotChocolate/Core/src/Validation/DocumentValidator.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public sealed class DocumentValidator
1818
private readonly IDocumentValidatorRule[] _nonCacheableRules;
1919
private readonly int _maxAllowedErrors;
2020
private readonly int _maxLocationsPerError;
21+
private readonly int _maxAllowedFragmentVisits;
2122

2223
/// <summary>
2324
/// Initializes a new instance of <see cref="DocumentValidator"/>.
@@ -34,11 +35,15 @@ public sealed class DocumentValidator
3435
/// <param name="maxLocationsPerError">
3536
/// The maximum number of locations that will be added to a validation error.
3637
/// </param>
38+
/// <param name="maxAllowedFragmentVisits">
39+
/// The maximum number of fragment visits allowed during validation.
40+
/// </param>
3741
internal DocumentValidator(
3842
ObjectPool<DocumentValidatorContext> contextPool,
3943
IDocumentValidatorRule[] rules,
4044
int maxAllowedErrors,
41-
int maxLocationsPerError)
45+
int maxLocationsPerError,
46+
int maxAllowedFragmentVisits)
4247
{
4348
ArgumentNullException.ThrowIfNull(rules);
4449
ArgumentNullException.ThrowIfNull(contextPool);
@@ -49,6 +54,7 @@ internal DocumentValidator(
4954
_nonCacheableRules = [.. rules.Where(rule => !rule.IsCacheable)];
5055
_maxAllowedErrors = maxAllowedErrors > 0 ? maxAllowedErrors : 1;
5156
_maxLocationsPerError = maxLocationsPerError > 0 ? maxLocationsPerError : 1;
57+
_maxAllowedFragmentVisits = maxAllowedFragmentVisits > 0 ? maxAllowedFragmentVisits : 1;
5258
}
5359

5460
/// <summary>
@@ -158,7 +164,7 @@ private DocumentValidatorContext RentContext(
158164
IFeatureCollection? features)
159165
{
160166
var context = _contextPool.Get();
161-
context.Initialize(schema, documentId, document, _maxAllowedErrors, _maxLocationsPerError, features);
167+
context.Initialize(schema, documentId, document, _maxAllowedErrors, _maxLocationsPerError, _maxAllowedFragmentVisits, features);
162168
return context;
163169
}
164170

src/HotChocolate/Core/src/Validation/DocumentValidatorBuilder.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,8 @@ public DocumentValidator Build()
229229
contextPool,
230230
[.. rules],
231231
_options.MaxAllowedErrors,
232-
_options.MaxLocationsPerError);
232+
_options.MaxLocationsPerError,
233+
_options.MaxAllowedFragmentVisits);
233234
}
234235

235236
private static T CreateInstance<T>(

src/HotChocolate/Core/src/Validation/DocumentValidatorContext.cs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ public void Initialize(
144144
DocumentNode document,
145145
int maxAllowedErrors,
146146
int maxLocationsPerError,
147+
int maxAllowedFragmentVisits,
147148
IFeatureCollection? features)
148149
{
149150
ArgumentNullException.ThrowIfNull(schema);
@@ -156,6 +157,8 @@ public void Initialize(
156157
_maxAllowedErrors = maxAllowedErrors;
157158
_maxLocationsPerError = maxLocationsPerError;
158159

160+
Fragments.SetMaxAllowedFragmentVisits(maxAllowedFragmentVisits);
161+
159162
_features.Initialize(features);
160163

161164
foreach (var definitionNode in document.Definitions)
@@ -247,6 +250,8 @@ public sealed class FragmentContext
247250
{
248251
private readonly HashSet<string> _visited = [];
249252
private readonly Dictionary<string, FragmentDefinitionNode> _fragments = new(StringComparer.Ordinal);
253+
private int _maxAllowedFragmentVisits;
254+
private int _fragmentVisits;
250255

251256
public IEnumerable<string> Names => _fragments.Keys;
252257

@@ -267,9 +272,16 @@ public bool TryGet(FragmentSpreadNode spread, [NotNullWhen(true)] out FragmentDe
267272

268273
public bool TryEnter(FragmentSpreadNode spread, [NotNullWhen(true)] out FragmentDefinitionNode? fragment)
269274
{
275+
if (_fragmentVisits >= _maxAllowedFragmentVisits)
276+
{
277+
fragment = null;
278+
return false;
279+
}
280+
270281
if (_visited.Add(spread.Name.Value)
271282
&& _fragments.TryGetValue(spread.Name.Value, out fragment))
272283
{
284+
_fragmentVisits++;
273285
return true;
274286
}
275287

@@ -286,13 +298,22 @@ public void Leave(FragmentDefinitionNode fragment)
286298
public bool Exists(FragmentSpreadNode spread)
287299
=> _fragments.ContainsKey(spread.Name.Value);
288300

301+
internal void SetMaxAllowedFragmentVisits(int maxAllowedFragmentVisits)
302+
{
303+
_maxAllowedFragmentVisits = maxAllowedFragmentVisits;
304+
}
305+
289306
internal void Reset()
290-
=> _visited.Clear();
307+
{
308+
_visited.Clear();
309+
_fragmentVisits = 0;
310+
}
291311

292312
internal void Clear()
293313
{
294314
_visited.Clear();
295315
_fragments.Clear();
316+
_fragmentVisits = 0;
296317
}
297318
}
298319
}

src/HotChocolate/Core/src/Validation/Extensions/ValidationBuilderExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public static DocumentValidatorBuilder AddFieldRules(
119119
return builder
120120
.AddRule<FieldSelectionsRule>()
121121
.AddRule<LeafFieldSelectionsRule>()
122-
.AddRule<OverlappingFieldsCanBeMergedRule>();
122+
.AddRule((_, o) => new OverlappingFieldsCanBeMergedRule(o.MaxAllowedFieldMergeComparisons));
123123
}
124124

125125
/// <summary>

src/HotChocolate/Core/src/Validation/Options/ValidationOptions.cs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,50 @@ public ushort MaxAllowedListRecursiveDepth
8080
get;
8181
set => field = value > 0 ? value : (ushort)16;
8282
} = 1;
83+
84+
/// <summary>
85+
/// <para>
86+
/// The maximum number of field-merge comparisons allowed during
87+
/// overlapping-fields-can-be-merged validation. This prevents
88+
/// adversarial queries with deeply nested inline fragments from
89+
/// consuming unbounded CPU.
90+
/// </para>
91+
/// <para>Default: <c>100,000</c></para>
92+
/// </summary>
93+
public int MaxAllowedFieldMergeComparisons
94+
{
95+
get;
96+
set
97+
{
98+
if (value < 1)
99+
{
100+
value = 100_000;
101+
}
102+
103+
field = value;
104+
}
105+
} = 100_000;
106+
107+
/// <summary>
108+
/// <para>
109+
/// The maximum number of fragment visits allowed during validation.
110+
/// Each time a fragment spread is entered counts as one visit.
111+
/// This prevents adversarial queries with deeply nested or repeated
112+
/// fragment spreads from consuming unbounded CPU.
113+
/// </para>
114+
/// <para>Default: <c>1,000</c></para>
115+
/// </summary>
116+
public int MaxAllowedFragmentVisits
117+
{
118+
get;
119+
set
120+
{
121+
if (value < 1)
122+
{
123+
value = 1_000;
124+
}
125+
126+
field = value;
127+
}
128+
} = 1_000;
83129
}

0 commit comments

Comments
 (0)