Skip to content

fix: bound YAML anchor/alias expansion to prevent OOM (billion laughs) - #3000

Open
Treicy Sanchez (Treicysg) wants to merge 2 commits into
microsoft:mainfrom
Treicysg:fix/yaml-alias-expansion-dos
Open

fix: bound YAML anchor/alias expansion to prevent OOM (billion laughs)#3000
Treicy Sanchez (Treicysg) wants to merge 2 commits into
microsoft:mainfrom
Treicysg:fix/yaml-alias-expansion-dos

Conversation

@Treicysg

@TreicysgTreicy Sanchez (Treicysg) commented Aug 7, 2026

Copy link
Copy Markdown

Problem

The YAML reader is exposed to an uncontrolled-resource-consumption ("billion laughs") denial of service (CWE-400). A tiny YAML document (well under 1 KB) using nested anchors/aliases can force the process to allocate many gigabytes and be OOM-killed. See https://portal.microsofticm.com/imp/v5/incidents/details/31000000667626/summary

Root cause

OpenApiYamlReader parses YAML via SharpYaml's YamlStream.Load, which produces a DAG where an alias (*a) resolves to the same shared YamlNode instance — so the parsed graph stays small. The explosion happens in YamlConverter.ToJsonNode, which converts that DAG into a System.Text.JsonJsonNode tree. Because JsonNode is single-parent (a node cannot be attached to two parents), every alias occurrence must be materialized as an independent copy. With no bound, N nested anchors each referenced k times expand to k^N nodes → exponential memory → OOM.

Deduplication/sharing is not possible (single-parent constraint), so the only viable fix is to bound the work and fail fast.

Fix

Add a conversion budget threaded through YamlConverter.ToJsonNode that enforces two limits:

LimitDefaultProtects against
Max materialized node count5,000,000Exponential anchor/alias expansion — the counter measures expanded nodes, so it trips well before OOM
Max nesting depth64Deep-nesting stack overflow in the recursive converter

Rationale for the values:

  • Depth 64 mirrors the default System.Text.JsonMaxDepth already enforced on the JSON reader path (JsonNode.Parse), so this brings YAML to parity — any document deeper than 64 already fails today when supplied as JSON.
  • Node count 5,000,000 is comfortably above legitimate specs (whose node count is linear in document size when they don't rely on alias fan-out), while a bomb trips almost immediately because the counter measures the expansion.

On breach the budget throws OpenApiReaderException, which OpenApiYamlReader.Read converts into an OpenApiDiagnosticerror (Document = null) — consistent with the existing JsonException handling — instead of allowing an OOM.

Configurable limits

The two limits are exposed as public static properties on YamlConverter so consumers are never blocked by the defaults:

  • YamlConverter.MaxDepth (default YamlConverter.DefaultMaxDepth = 64)
  • YamlConverter.MaxNodeCount (default YamlConverter.DefaultMaxNodeCount = 5,000,000)

A consumer that must ingest an unusually large/deep-but-trusted document can raise the limits; a consumer that only ever parses small documents can lower them to fail faster. Setters validate the value is greater than zero. These are the only additions to the public API (recorded in PublicAPI.Unshipped.txt).

Tests

  • YamlConverterTests.ExponentialAliasExpansionIsRejected — a nested anchor/alias bomb is rejected instead of exhausting memory.
  • YamlConverterTests.ExcessiveNestingDepthIsRejected — nesting beyond the depth limit is rejected.
  • YamlConverterTests.LegitimateAliasesStillConvert — normal alias usage still converts correctly.
  • YamlConverterTests.ConversionLimitsDefaultToDocumentedValues — the properties expose the documented defaults.
  • YamlConverterTests.SettingMaxDepthBelowOneThrows / SettingMaxNodeCountBelowOneThrows — invalid limits are rejected and leave the effective limit unchanged.
  • YamlConverterTests.RaisingMaxDepthAllowsDocumentsDeeperThanTheDefault — raising the limit permits a document deeper than the default.
  • OpenApiYamlReaderTests.ReadReturnsDiagnosticErrorForExponentialAliasExpansion — the reader surfaces a diagnostic error (no document), not a throw/OOM.

Full Microsoft.OpenApi.Readers.Tests suite passes (599 tests).

Validation on a large real-world spec (no false positives)

To confirm the limits do not reject legitimately large production descriptions, the Microsoft Graph beta OpenAPI document was loaded end-to-end through the patched reader.

PropertyValue
Sourcemicrosoftgraph/msgraph-metadata @ 73fc270c924975a98f8f9d93d61fd4cff2297084, path openapi/beta/openapi.yaml
SHA-256830108DDB021845583F0C9F6D185BCE465CA4CB1E673E50B2F98DB05E54C21AD
Size66.4 MB (69,627,334 bytes), OpenAPI 3.0.4
Content18,485 paths, 10,368 component schemas

Result under the patched build:

MetricMeasuredLimitUtilization
Materialized JSON nodes1,735,8555,000,000~35% (≈2.9× headroom)
Max nesting depth1464~22% (≈4.5× headroom)
OpenApiDocument.LoadAsyncsuccess, 0 diagnostics

The largest realistic production spec sits well below both caps, while the exponential bomb (theoretical ~387M nodes) is rejected almost immediately. The limits target the exponential pathology, not document size.

Notes / scope

  • This is distinct from CVE-2026-49451 (circular $ref stack overflow), which was already fixed in 3.5.4.
  • ReadFragment is still protected by the budget (it throws rather than OOM) but does not convert the exception to a diagnostic — left out of scope intentionally; happy to extend if preferred.
  • The same fix applies byte-for-byte to support/v2; a companion PR will follow. support/v1 uses a different YAML parsing path and needs a separate assessment.

The YAML reader converts the SharpYaml node graph - a DAG in which aliases
share a single instance - into a System.Text.Json JsonNode tree, allocating
a fresh node per path. Because JsonNode is single-parent, shared aliases must
be duplicated, so a tiny document with nested anchors/aliases expands
exponentially and exhausts process memory (CWE-400, uncontrolled resource
consumption).
Add a conversion budget to YamlConverter.ToJsonNode that caps the total
materialized node count (5,000,000) and nesting depth (64, mirroring the
System.Text.Json default already enforced on the JSON reader path). On breach
it throws OpenApiReaderException, which OpenApiYamlReader.Read converts into an
OpenApiDiagnostic error instead of allowing an OOM. Public API is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 022bbd4f-e5e7-447a-bcdf-b2a4efaf75c3
@Treicysg
Treicy Sanchez (Treicysg) requested a review from a team as a code ownerAugust 7, 2026 17:05
/// 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 

Expose YamlConverter.MaxDepth and MaxNodeCount as public static properties
(defaulting to DefaultMaxDepth=64 and DefaultMaxNodeCount=5,000,000) so
consumers can raise the limits for legitimately large/deep documents or lower
them to fail faster on known-small inputs, without needing a library change.
Setters validate that the value is greater than zero. Public API entries added.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 022bbd4f-e5e7-447a-bcdf-b2a4efaf75c3
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Treicysg@gavinbarron