Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/Microsoft.OpenApi.YamlReader/OpenApiYamlReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ public ReadResult Read(MemoryStream input,
Diagnostic = diagnostic,
};
}
catch (OpenApiReaderException ex)
{
var diagnostic = new OpenApiDiagnostic();
diagnostic.Errors.Add(new(ex));
diagnostic.Format = OpenApiConstants.Yaml;
return new()
{
Document = null,
Diagnostic = diagnostic,
};
}

return UpdateFormat(Read(jsonNode, location, settings));
}
Expand Down
69 changes: 65 additions & 4 deletions src/Microsoft.OpenApi.YamlReader/YamlConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,51 @@ namespace Microsoft.OpenApi.YamlReader
/// </summary>
public static class YamlConverter
{
/// <summary>
/// Default maximum nesting depth allowed when converting a YAML node graph into JSON nodes.
/// Mirrors the default System.Text.Json depth limit (64) that already bounds the JSON reader path,
/// protecting the recursive conversion from stack exhaustion on deeply nested documents.
/// </summary>
internal const int DefaultMaxDepth = 64;

/// <summary>
/// Default maximum number of JSON nodes that may be materialized from a single YAML document.
/// Guards against YAML anchor/alias expansion ("billion laughs") attacks, where a tiny document
/// expands exponentially when its shared node graph is materialized into an independent JSON tree.
/// Increase this only if legitimate large documents are being rejected.
/// </summary>
internal const int DefaultMaxNodeCount = 5_000_000;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we expose these as publicly modifiable fields using these defaults so that a legitimate consumption scenario that needs to go deeper or larger has a mechanism to do so without needing to file a bug on us here?

Actually, this would also allow a consumer to set smaller limits and fail faster if they knew they had small documents to parse

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exposed both limits as public static properties ( DefaultMaxNodeCount  = 5,000,000, DefaultMaxDepth  = 64).
Also, setters validate  > 0 


/// <summary>
/// Tracks and enforces resource limits while converting a YAML node graph into JSON nodes,
/// failing fast when a hostile document would otherwise exhaust memory or the stack.
/// </summary>
private sealed class YamlConversionBudget
{
private readonly int _maxDepth;
private readonly int _maxNodeCount;
private int _nodeCount;

public YamlConversionBudget(int maxDepth = DefaultMaxDepth, int maxNodeCount = DefaultMaxNodeCount)
{
_maxDepth = maxDepth;
_maxNodeCount = maxNodeCount;
}

public void EnterNode(int depth)
{
if (depth > _maxDepth)
{
throw new OpenApiReaderException($"The YAML document exceeds the maximum supported nesting depth of {_maxDepth}.");
}

if (++_nodeCount > _maxNodeCount)
{
throw new OpenApiReaderException($"The YAML document expands to more than the maximum supported number of nodes ({_maxNodeCount}). This may indicate a YAML anchor/alias expansion (billion laughs) attack.");
}
}
}

/// <summary>
/// Converts all of the documents in a YAML stream to <see cref="JsonNode"/>s.
/// </summary>
Expand Down Expand Up @@ -42,10 +87,16 @@ public static JsonNode ToJsonNode(this YamlDocument yaml)
/// <exception cref="NotSupportedException">Thrown for YAML that is not compatible with JSON.</exception>
public static JsonNode ToJsonNode(this YamlNode yaml)
{
return yaml.ToJsonNode(new YamlConversionBudget(), 0);
}

private static JsonNode ToJsonNode(this YamlNode yaml, YamlConversionBudget budget, int depth)
{
budget.EnterNode(depth);
return yaml switch
{
YamlMappingNode map => map.ToJsonObject(),
YamlSequenceNode seq => seq.ToJsonArray(),
YamlMappingNode map => map.ToJsonObject(budget, depth),
YamlSequenceNode seq => seq.ToJsonArray(budget, depth),
YamlScalarNode scalar => scalar.ToJsonValue(),
_ => throw new NotSupportedException("This yaml isn't convertible to JSON")
};
Expand Down Expand Up @@ -78,12 +129,17 @@ public static YamlNode ToYamlNode(this JsonNode json)
/// <param name="yaml"></param>
/// <returns></returns>
public static JsonObject ToJsonObject(this YamlMappingNode yaml)
{
return yaml.ToJsonObject(new YamlConversionBudget(), 0);
}

private static JsonObject ToJsonObject(this YamlMappingNode yaml, YamlConversionBudget budget, int depth)
{
var node = new JsonObject();
foreach (var keyValuePair in yaml)
{
var key = ((YamlScalarNode)keyValuePair.Key).Value!;
node[key] = keyValuePair.Value.ToJsonNode();
node[key] = keyValuePair.Value.ToJsonNode(budget, depth + 1);
}

return node;
Expand All @@ -103,11 +159,16 @@ private static YamlMappingNode ToYamlMapping(this JsonObject obj)
/// <param name="yaml"></param>
/// <returns></returns>
public static JsonArray ToJsonArray(this YamlSequenceNode yaml)
{
return yaml.ToJsonArray(new YamlConversionBudget(), 0);
}

private static JsonArray ToJsonArray(this YamlSequenceNode yaml, YamlConversionBudget budget, int depth)
{
var node = new JsonArray();
foreach (var value in yaml)
{
node.Add(value.ToJsonNode());
node.Add(value.ToJsonNode(budget, depth + 1));
}

return node;
Expand Down
26 changes: 26 additions & 0 deletions test/Microsoft.OpenApi.Readers.Tests/OpenApiYamlReaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,32 @@ public void ReadThrowsWhenSettingsIsNull()
Assert.Throws<ArgumentNullException>(() => reader.Read(stream, DocumentLocation, null!));
}

[Fact]
public void ReadReturnsDiagnosticErrorForExponentialAliasExpansion()
{
// A "billion laughs" YAML bomb must surface as a diagnostic error with no document,
// rather than throwing or exhausting memory.
var reader = new OpenApiYamlReader();
using var stream = CreateStream(
"""
a: &a ["x","x","x","x","x","x","x","x","x"]
b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a]
c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b]
d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c]
e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d]
f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e]
g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f]
h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g]
i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h]
""");

var result = reader.Read(stream, DocumentLocation, SettingsFixture.ReaderSettings);

Assert.Null(result.Document);
Assert.NotEmpty(result.Diagnostic.Errors);
Assert.Equal(OpenApiConstants.Yaml, result.Diagnostic.Format);
}

private static MemoryStream CreateStream(string yaml)
{
return new MemoryStream(Encoding.UTF8.GetBytes(yaml));
Expand Down
48 changes: 48 additions & 0 deletions test/Microsoft.OpenApi.Readers.Tests/YamlConverterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,54 @@ public void RoundTripEmptyStringsValues()
Assert.Equal(yamlInput.MakeLineBreaksEnvironmentNeutral(), convertedBackOutput.MakeLineBreaksEnvironmentNeutral());
}

[Fact]
public void ExponentialAliasExpansionIsRejected()
{
// A "billion laughs" YAML bomb: each level references the previous one multiple times,
// so materializing the shared node graph into an independent JSON tree expands
// exponentially. The conversion must fail fast instead of exhausting memory.
var yamlBomb =
"""
a: &a ["x","x","x","x","x","x","x","x","x"]
b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a]
c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b]
d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c]
e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d]
f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e]
g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f]
h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g]
i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h]
""";

Assert.Throws<OpenApiReaderException>(() => ConvertYamlStringToJsonNode(yamlBomb));
}

[Fact]
public void ExcessiveNestingDepthIsRejected()
{
// Deeper than the conversion depth limit (mirrors the System.Text.Json default of 64),
// which protects the recursive converter from stack exhaustion.
const int depth = 70;
var deeplyNested = new string('[', depth) + new string(']', depth);

Assert.Throws<OpenApiReaderException>(() => ConvertYamlStringToJsonNode(deeplyNested));
}

[Fact]
public void LegitimateAliasesStillConvert()
{
var yamlInput =
"""
a: &val hello
b: *val
""";

var jsonNode = Assert.IsType<JsonObject>(ConvertYamlStringToJsonNode(yamlInput));

Assert.Equal("hello", jsonNode["a"]?.GetValue<string>());
Assert.Equal("hello", jsonNode["b"]?.GetValue<string>());
}

private static JsonNode ConvertYamlStringToJsonNode(string yamlInput)
{
var yamlDocument = new YamlStream();
Expand Down