From 5fd5a50be95adc3625bbcccc37035b068f777e6f Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Sat, 28 Sep 2024 00:39:51 +0200 Subject: [PATCH] Added Introspection Cycle Detection --- .github/workflows/release.yml | 2 +- .../Core/src/Abstractions/ErrorCodes.cs | 15 ++- .../Core/src/Validation/CoordinateLimit.cs | 27 ++++++ .../Core/src/Validation/DocumentValidator.cs | 5 + .../Validation/DocumentValidatorContext.cs | 6 ++ .../Core/src/Validation/ErrorHelper.cs | 14 +++ .../ValidatiobBuilderExtensions.Rules.cs | 7 ++ .../ValidationServiceCollectionExtensions.cs | 1 + .../src/Validation/FieldDepthCycleTracker.cs | 90 ++++++++++++++++++ .../Validation/IDocumentValidatorContext.cs | 10 ++ .../Rules/IntrospectionDepthVisitor.cs | 79 ++++++++++++++++ .../DocumentValidatorTests.cs | 6 ++ .../introspection_with_cycle.graphql | 93 +++++++++++++++++++ ...torTests.Introspection_Cycle_Detected.snap | 86 +++++++++++++++++ 14 files changed, 435 insertions(+), 6 deletions(-) create mode 100644 src/HotChocolate/Core/src/Validation/CoordinateLimit.cs create mode 100644 src/HotChocolate/Core/src/Validation/FieldDepthCycleTracker.cs create mode 100644 src/HotChocolate/Core/src/Validation/Rules/IntrospectionDepthVisitor.cs create mode 100644 src/HotChocolate/Core/test/Validation.Tests/__resources__/introspection_with_cycle.graphql create mode 100644 src/HotChocolate/Core/test/Validation.Tests/__snapshots__/DocumentValidatorTests.Introspection_Cycle_Detected.snap diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 70f0af46aea..9cfc43a6d74 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -7,7 +7,7 @@ on: jobs: release: - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 steps: - name: Checkout uses: actions/checkout@v3 diff --git a/src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs b/src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs index 6301c5892d8..86e1adfb3b2 100644 --- a/src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs +++ b/src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs @@ -65,21 +65,21 @@ public static class Execution /// /// The Oneof Input Objects `{0}` require that exactly one field must be supplied and that - /// field must not be `null`. Oneof Input Objects are a special variant of Input Objects + /// field must not be `null`. Oneof Input Objects are a special variant of Input Objects /// where the type system asserts that exactly one of the fields must be set and non-null. /// public const string OneOfNoFieldSet = "HC0054"; /// - /// More than one field of the Oneof Input Object `{0}` is set. Oneof Input Objects - /// are a special variant of Input Objects where the type system asserts that exactly + /// More than one field of the Oneof Input Object `{0}` is set. Oneof Input Objects + /// are a special variant of Input Objects where the type system asserts that exactly /// one of the fields must be set and non-null. /// public const string OneOfMoreThanOneFieldSet = "HC0055"; /// - /// `null` was set to the field `{0}`of the Oneof Input Object `{1}`. Oneof Input Objects - /// are a special variant of Input Objects where the type system asserts that exactly + /// `null` was set to the field `{0}`of the Oneof Input Object `{1}`. Oneof Input Objects + /// are a special variant of Input Objects where the type system asserts that exactly /// one of the fields must be set and non-null. /// public const string OneOfFieldIsNull = "HC0056"; @@ -268,6 +268,11 @@ public static class Validation /// The introspection is not allowed for the current request /// public const string IntrospectionNotAllowed = "HC0046"; + + /// + /// The maximum allowed introspection depth was exceeded. + /// + public const string MaxIntrospectionDepthOverflow = "HC0086"; } /// diff --git a/src/HotChocolate/Core/src/Validation/CoordinateLimit.cs b/src/HotChocolate/Core/src/Validation/CoordinateLimit.cs new file mode 100644 index 00000000000..2717b7cc7bd --- /dev/null +++ b/src/HotChocolate/Core/src/Validation/CoordinateLimit.cs @@ -0,0 +1,27 @@ +namespace HotChocolate.Validation; + +internal sealed class CoordinateLimit +{ + public ushort MaxAllowed { get; private set; } + + public ushort Count { get; private set; } + + public bool Add() + { + if (Count < MaxAllowed) + { + Count++; + return true; + } + + return false; + } + + public void Remove() => Count--; + + public void Reset(ushort maxAllowed) + { + MaxAllowed = maxAllowed; + Count = 0; + } +} diff --git a/src/HotChocolate/Core/src/Validation/DocumentValidator.cs b/src/HotChocolate/Core/src/Validation/DocumentValidator.cs index 4ecf6f2dd65..4de0a15ba51 100644 --- a/src/HotChocolate/Core/src/Validation/DocumentValidator.cs +++ b/src/HotChocolate/Core/src/Validation/DocumentValidator.cs @@ -89,6 +89,11 @@ public DocumentValidatorResult Validate( foreach (IDocumentValidatorRule? rule in rules) { rule.Validate(context, document); + + if (context.FatalErrorDetected) + { + break; + } } return context.Errors.Count > 0 diff --git a/src/HotChocolate/Core/src/Validation/DocumentValidatorContext.cs b/src/HotChocolate/Core/src/Validation/DocumentValidatorContext.cs index 643cc1f8198..506cb89d5d2 100644 --- a/src/HotChocolate/Core/src/Validation/DocumentValidatorContext.cs +++ b/src/HotChocolate/Core/src/Validation/DocumentValidatorContext.cs @@ -94,6 +94,8 @@ public IOutputType NonNullString public bool UnexpectedErrorsDetected { get; set; } + public bool FatalErrorDetected { get; set; } + public int Count { get; set; } public int Max { get; set; } @@ -106,6 +108,8 @@ public IOutputType NonNullString public HashSet ProcessedFieldPairs { get; } = new(); + public FieldDepthCycleTracker FieldDepth { get; } = new(); + public IList RentFieldInfoList() { FieldInfoListBuffer buffer = _buffers.Peek(); @@ -159,7 +163,9 @@ public void Clear() InputFields.Clear(); _errors.Clear(); List.Clear(); + FieldDepth.Reset(); UnexpectedErrorsDetected = false; + FatalErrorDetected = false; Count = 0; Max = 0; MaxAllowedErrors = 0; diff --git a/src/HotChocolate/Core/src/Validation/ErrorHelper.cs b/src/HotChocolate/Core/src/Validation/ErrorHelper.cs index 6aeeeb84760..dacb45a31af 100644 --- a/src/HotChocolate/Core/src/Validation/ErrorHelper.cs +++ b/src/HotChocolate/Core/src/Validation/ErrorHelper.cs @@ -685,4 +685,18 @@ public static IError OneOfVariablesMustBeNonNull( .SetExtension(nameof(field), field.ToString()) .SpecifiedBy("sec-Oneof–Input-Objects-Have-Exactly-One-Field", rfc: 825) .Build(); + + public static void ReportMaxIntrospectionDepthOverflow( + this IDocumentValidatorContext context, + ISyntaxNode selection) + { + context.FatalErrorDetected = true; + context.ReportError( + ErrorBuilder.New() + .SetMessage("Maximum allowed introspection depth exceeded.") + .SetCode(ErrorCodes.Validation.MaxIntrospectionDepthOverflow) + .SetSyntaxNode(selection) + .SetPath(context.CreateErrorPath()) + .Build()); + } } diff --git a/src/HotChocolate/Core/src/Validation/Extensions/ValidatiobBuilderExtensions.Rules.cs b/src/HotChocolate/Core/src/Validation/Extensions/ValidatiobBuilderExtensions.Rules.cs index 1353d3c97c9..c55600e1914 100644 --- a/src/HotChocolate/Core/src/Validation/Extensions/ValidatiobBuilderExtensions.Rules.cs +++ b/src/HotChocolate/Core/src/Validation/Extensions/ValidatiobBuilderExtensions.Rules.cs @@ -326,4 +326,11 @@ public static IValidationBuilder AddMaxExecutionDepthRule( public static IValidationBuilder AddIntrospectionAllowedRule( this IValidationBuilder builder) => builder.TryAddValidationVisitor((_, _) => new IntrospectionVisitor(), false); + + /// + /// Adds a validation rule that restricts the depth of a GraphQL introspection request. + /// + public static IValidationBuilder AddIntrospectionDepthRule( + this IValidationBuilder builder) + => builder.TryAddValidationVisitor(); } diff --git a/src/HotChocolate/Core/src/Validation/Extensions/ValidationServiceCollectionExtensions.cs b/src/HotChocolate/Core/src/Validation/Extensions/ValidationServiceCollectionExtensions.cs index 25fbb1fb259..c25cf87a98f 100644 --- a/src/HotChocolate/Core/src/Validation/Extensions/ValidationServiceCollectionExtensions.cs +++ b/src/HotChocolate/Core/src/Validation/Extensions/ValidationServiceCollectionExtensions.cs @@ -28,6 +28,7 @@ public static IValidationBuilder AddValidation( var builder = new DefaultValidationBuilder(schemaName, services); builder + .AddIntrospectionDepthRule() .AddDocumentRules() .AddOperationRules() .AddFieldRules() diff --git a/src/HotChocolate/Core/src/Validation/FieldDepthCycleTracker.cs b/src/HotChocolate/Core/src/Validation/FieldDepthCycleTracker.cs new file mode 100644 index 00000000000..f0321bb1b7f --- /dev/null +++ b/src/HotChocolate/Core/src/Validation/FieldDepthCycleTracker.cs @@ -0,0 +1,90 @@ +using System.Collections.Generic; +using HotChocolate.Language; + +namespace HotChocolate.Validation; + +/// +/// Allows to track field cycle depths in a GraphQL query. +/// +public sealed class FieldDepthCycleTracker +{ + private readonly Dictionary _coordinates = new(); + private readonly List _limits = new(); + private ushort? _defaultMaxAllowed; + + /// + /// Adds a field coordinate to the tracker. + /// + /// + /// A field coordinate. + /// + /// + /// true if the field coordinate has not reached its cycle depth limit; + /// otherwise, false. + /// + public bool Add(FieldCoordinate coordinate) + { + if (_coordinates.TryGetValue(coordinate, out var limit)) + { + return limit.Add(); + } + + if(_defaultMaxAllowed.HasValue) + { + _limits.TryPop(out limit); + limit ??= new CoordinateLimit(); + limit.Reset(_defaultMaxAllowed.Value); + _coordinates.Add(coordinate, limit); + return limit.Add(); + } + + return true; + } + + /// + /// Removes a field coordinate from the tracker. + /// + /// + /// A field coordinate. + /// + public void Remove(FieldCoordinate coordinate) + { + if (_coordinates.TryGetValue(coordinate, out var limit)) + { + limit.Remove(); + } + } + + /// + /// Initializes the field depth tracker with the specified limits. + /// + /// + /// A collection of field coordinates and their cycle depth limits. + /// + /// + /// The default cycle depth limit for coordinates that were not explicitly defined. + /// + public void Initialize( + IEnumerable<(FieldCoordinate Coordinate, ushort MaxAllowed)> limits, + ushort? defaultMaxAllowed = null) + { + foreach (var (coordinate, maxAllowed) in limits) + { + _limits.TryPop(out var limit); + limit ??= new CoordinateLimit(); + limit.Reset(maxAllowed); + _coordinates.Add(coordinate, limit); + } + + _defaultMaxAllowed = defaultMaxAllowed; + } + + /// + /// Resets the field depth tracker. + /// + public void Reset() + { + _limits.AddRange(_coordinates.Values); + _coordinates.Clear(); + } +} diff --git a/src/HotChocolate/Core/src/Validation/IDocumentValidatorContext.cs b/src/HotChocolate/Core/src/Validation/IDocumentValidatorContext.cs index 15486932a79..1e0aea86ed2 100644 --- a/src/HotChocolate/Core/src/Validation/IDocumentValidatorContext.cs +++ b/src/HotChocolate/Core/src/Validation/IDocumentValidatorContext.cs @@ -140,6 +140,11 @@ public interface IDocumentValidatorContext : ISyntaxVisitorContext /// bool UnexpectedErrorsDetected { get; set; } + /// + /// Defines that a fatal error was detected and that the analyzer will be aborted. + /// + bool FatalErrorDetected { get; set; } + /// /// A map to store arbitrary visitor data. /// @@ -160,6 +165,11 @@ public interface IDocumentValidatorContext : ISyntaxVisitorContext /// HashSet ProcessedFieldPairs { get; } + /// + /// Gets the field depth cycle tracker. + /// + FieldDepthCycleTracker FieldDepth { get; } + /// /// Rents a list of field infos. /// diff --git a/src/HotChocolate/Core/src/Validation/Rules/IntrospectionDepthVisitor.cs b/src/HotChocolate/Core/src/Validation/Rules/IntrospectionDepthVisitor.cs new file mode 100644 index 00000000000..777de38fb06 --- /dev/null +++ b/src/HotChocolate/Core/src/Validation/Rules/IntrospectionDepthVisitor.cs @@ -0,0 +1,79 @@ +using System; +using HotChocolate.Language; +using HotChocolate.Language.Visitors; +using HotChocolate.Types; +using HotChocolate.Types.Introspection; +using HotChocolate.Utilities; + +namespace HotChocolate.Validation.Rules; + +/// +/// This rules ensures that recursive introspection fields cannot be used +/// to create endless cycles. +/// +internal sealed class IntrospectionDepthVisitor : TypeDocumentValidatorVisitor +{ + private readonly (FieldCoordinate Coordinate, ushort MaxAllowed)[] _limits = + new (FieldCoordinate Coordinate, ushort MaxAllowed)[] + { + (new FieldCoordinate("__Type", "fields"), 1), + (new FieldCoordinate("__Type", "inputFields"), 1), + (new FieldCoordinate("__Type", "interfaces"), 1), + (new FieldCoordinate("__Type", "possibleTypes"), 1), + (new FieldCoordinate("__Type", "ofType"), 8), + }; + + protected override ISyntaxVisitorAction Enter( + DocumentNode node, + IDocumentValidatorContext context) + { + context.FieldDepth.Initialize(_limits); + return base.Enter(node, context); + } + + protected override ISyntaxVisitorAction Enter( + FieldNode node, + IDocumentValidatorContext context) + { + if (IntrospectionFields.TypeName.Equals(node.Name.Value, StringComparison.Ordinal)) + { + return Skip; + } + + if (context.Types.TryPeek(out var type) + && type.NamedType() is IComplexOutputType ot + && ot.Fields.TryGetField(node.Name.Value, out var of)) + { + // we are only interested in fields if the root field is either + // __type or __schema. + if (context.OutputFields.Count == 0 + && !of.IsIntrospectionField) + { + return Skip; + } + + if (!context.FieldDepth.Add(of.Coordinate)) + { + context.ReportMaxIntrospectionDepthOverflow(node); + return Break; + } + + context.OutputFields.Push(of); + context.Types.Push(of.Type); + return Continue; + } + + context.UnexpectedErrorsDetected = true; + return Skip; + } + + protected override ISyntaxVisitorAction Leave( + FieldNode node, + IDocumentValidatorContext context) + { + context.FieldDepth.Remove(context.OutputFields.Peek().Coordinate); + context.Types.Pop(); + context.OutputFields.Pop(); + return Continue; + } +} diff --git a/src/HotChocolate/Core/test/Validation.Tests/DocumentValidatorTests.cs b/src/HotChocolate/Core/test/Validation.Tests/DocumentValidatorTests.cs index 01ef3b9f348..f098c27cede 100644 --- a/src/HotChocolate/Core/test/Validation.Tests/DocumentValidatorTests.cs +++ b/src/HotChocolate/Core/test/Validation.Tests/DocumentValidatorTests.cs @@ -813,6 +813,12 @@ public void Produce_Many_Errors_50000_query() ExpectErrors(FileResource.Open("50000_query.graphql")); } + [Fact] + public void Introspection_Cycle_Detected() + { + ExpectErrors(FileResource.Open("introspection_with_cycle.graphql")); + } + private void ExpectValid(string sourceText) => ExpectValid(null, null, sourceText); private void ExpectValid(ISchema schema, IDocumentValidator validator, string sourceText) diff --git a/src/HotChocolate/Core/test/Validation.Tests/__resources__/introspection_with_cycle.graphql b/src/HotChocolate/Core/test/Validation.Tests/__resources__/introspection_with_cycle.graphql new file mode 100644 index 00000000000..ac0a3e54237 --- /dev/null +++ b/src/HotChocolate/Core/test/Validation.Tests/__resources__/introspection_with_cycle.graphql @@ -0,0 +1,93 @@ +query IntrospectionQuery { + __schema { + queryType { + name + } + mutationType { + name + } + types { + ... FullType + } + directives { + name + description + args { + ... InputValue + } + onOperation + onFragment + onField + } + } +} + +fragment FullType on __Type { + kind + name + description + fields(includeDeprecated: true) { + name + description + args { + ... InputValue + } + type { + ... TypeRef + fields { + name + } + } + isDeprecated + deprecationReason + } + inputFields { + ... InputValue + } + interfaces { + ... TypeRef + } + enumValues(includeDeprecated: true) { + name + description + isDeprecated + deprecationReason + } + possibleTypes { + ... TypeRef + } +} + +fragment InputValue on __InputValue { + name + description + type { + ... TypeRef + } + defaultValue +} + +fragment TypeRef on __Type { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + } + } + } + } + } +} diff --git a/src/HotChocolate/Core/test/Validation.Tests/__snapshots__/DocumentValidatorTests.Introspection_Cycle_Detected.snap b/src/HotChocolate/Core/test/Validation.Tests/__snapshots__/DocumentValidatorTests.Introspection_Cycle_Detected.snap new file mode 100644 index 00000000000..b4cde2ff1de --- /dev/null +++ b/src/HotChocolate/Core/test/Validation.Tests/__snapshots__/DocumentValidatorTests.Introspection_Cycle_Detected.snap @@ -0,0 +1,86 @@ +[ + { + "Message": "Maximum allowed introspection depth exceeded.", + "Code": "HC0086", + "Path": { + "Parent": { + "Parent": { + "Parent": { + "Parent": null, + "Depth": 0, + "Name": "__schema" + }, + "Depth": 1, + "Name": "types" + }, + "Depth": 2, + "Name": "fields" + }, + "Depth": 3, + "Name": "type" + }, + "Locations": null, + "Extensions": { + "code": "HC0086" + }, + "Exception": null, + "SyntaxNode": { + "Kind": "Field", + "Alias": null, + "Arguments": [], + "Required": null, + "SelectionSet": { + "Kind": "SelectionSet", + "Location": { + "Start": 497, + "End": 525, + "Line": 37, + "Column": 14 + }, + "Selections": [ + { + "Kind": "Field", + "Alias": null, + "Arguments": [], + "Required": null, + "SelectionSet": null, + "Location": { + "Start": 507, + "End": 519, + "Line": 38, + "Column": 9 + }, + "Name": { + "Kind": "Name", + "Location": { + "Start": 507, + "End": 519, + "Line": 38, + "Column": 9 + }, + "Value": "name" + }, + "Directives": [] + } + ] + }, + "Location": { + "Start": 490, + "End": 525, + "Line": 37, + "Column": 7 + }, + "Name": { + "Kind": "Name", + "Location": { + "Start": 490, + "End": 498, + "Line": 37, + "Column": 7 + }, + "Value": "fields" + }, + "Directives": [] + } + } +]