Skip to content

Commit

Permalink
Added Introspection Cycle Detection
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelstaib committed Sep 27, 2024
1 parent fdb584c commit 5fd5a50
Show file tree
Hide file tree
Showing 14 changed files with 435 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ on:

jobs:
release:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
steps:
- name: Checkout
uses: actions/checkout@v3
Expand Down
15 changes: 10 additions & 5 deletions src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,21 @@ public static class Execution

/// <summary>
/// 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.
/// </summary>
public const string OneOfNoFieldSet = "HC0054";

/// <summary>
/// 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.
/// </summary>
public const string OneOfMoreThanOneFieldSet = "HC0055";

/// <summary>
/// `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.
/// </summary>
public const string OneOfFieldIsNull = "HC0056";
Expand Down Expand Up @@ -268,6 +268,11 @@ public static class Validation
/// The introspection is not allowed for the current request
/// </summary>
public const string IntrospectionNotAllowed = "HC0046";

/// <summary>
/// The maximum allowed introspection depth was exceeded.
/// </summary>
public const string MaxIntrospectionDepthOverflow = "HC0086";
}

/// <summary>
Expand Down
27 changes: 27 additions & 0 deletions src/HotChocolate/Core/src/Validation/CoordinateLimit.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
5 changes: 5 additions & 0 deletions src/HotChocolate/Core/src/Validation/DocumentValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -106,6 +108,8 @@ public IOutputType NonNullString

public HashSet<FieldInfoPair> ProcessedFieldPairs { get; } = new();

public FieldDepthCycleTracker FieldDepth { get; } = new();

public IList<FieldInfo> RentFieldInfoList()
{
FieldInfoListBuffer buffer = _buffers.Peek();
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 14 additions & 0 deletions src/HotChocolate/Core/src/Validation/ErrorHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -326,4 +326,11 @@ public static IValidationBuilder AddMaxExecutionDepthRule(
public static IValidationBuilder AddIntrospectionAllowedRule(
this IValidationBuilder builder) =>
builder.TryAddValidationVisitor((_, _) => new IntrospectionVisitor(), false);

/// <summary>
/// Adds a validation rule that restricts the depth of a GraphQL introspection request.
/// </summary>
public static IValidationBuilder AddIntrospectionDepthRule(
this IValidationBuilder builder)
=> builder.TryAddValidationVisitor<IntrospectionDepthVisitor>();
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public static IValidationBuilder AddValidation(
var builder = new DefaultValidationBuilder(schemaName, services);

builder
.AddIntrospectionDepthRule()
.AddDocumentRules()
.AddOperationRules()
.AddFieldRules()
Expand Down
90 changes: 90 additions & 0 deletions src/HotChocolate/Core/src/Validation/FieldDepthCycleTracker.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
using System.Collections.Generic;
using HotChocolate.Language;

namespace HotChocolate.Validation;

/// <summary>
/// Allows to track field cycle depths in a GraphQL query.
/// </summary>
public sealed class FieldDepthCycleTracker
{
private readonly Dictionary<FieldCoordinate, CoordinateLimit> _coordinates = new();
private readonly List<CoordinateLimit> _limits = new();
private ushort? _defaultMaxAllowed;

/// <summary>
/// Adds a field coordinate to the tracker.
/// </summary>
/// <param name="coordinate">
/// A field coordinate.
/// </param>
/// <returns>
/// <c>true</c> if the field coordinate has not reached its cycle depth limit;
/// otherwise, <c>false</c>.
/// </returns>
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;
}

/// <summary>
/// Removes a field coordinate from the tracker.
/// </summary>
/// <param name="coordinate">
/// A field coordinate.
/// </param>
public void Remove(FieldCoordinate coordinate)
{
if (_coordinates.TryGetValue(coordinate, out var limit))
{
limit.Remove();
}
}

/// <summary>
/// Initializes the field depth tracker with the specified limits.
/// </summary>
/// <param name="limits">
/// A collection of field coordinates and their cycle depth limits.
/// </param>
/// <param name="defaultMaxAllowed">
/// The default cycle depth limit for coordinates that were not explicitly defined.
/// </param>
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;
}

/// <summary>
/// Resets the field depth tracker.
/// </summary>
public void Reset()
{
_limits.AddRange(_coordinates.Values);
_coordinates.Clear();
}
}
10 changes: 10 additions & 0 deletions src/HotChocolate/Core/src/Validation/IDocumentValidatorContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ public interface IDocumentValidatorContext : ISyntaxVisitorContext
/// </summary>
bool UnexpectedErrorsDetected { get; set; }

/// <summary>
/// Defines that a fatal error was detected and that the analyzer will be aborted.
/// </summary>
bool FatalErrorDetected { get; set; }

/// <summary>
/// A map to store arbitrary visitor data.
/// </summary>
Expand All @@ -160,6 +165,11 @@ public interface IDocumentValidatorContext : ISyntaxVisitorContext
/// </summary>
HashSet<FieldInfoPair> ProcessedFieldPairs { get; }

/// <summary>
/// Gets the field depth cycle tracker.
/// </summary>
FieldDepthCycleTracker FieldDepth { get; }

/// <summary>
/// Rents a list of field infos.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

/// <summary>
/// This rules ensures that recursive introspection fields cannot be used
/// to create endless cycles.
/// </summary>
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 5fd5a50

Please sign in to comment.