From 5262ac4ab100653b87aad900fe8437d5e18511fa Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 16 Jun 2026 08:35:46 +0000 Subject: [PATCH 1/4] yaml: drop redundant merge-key property budget The per-stream merge_props_budget (1,048,576) added in #31417 rejected legitimate documents that merge a modest anchor into many mappings: a 380 KB file with 16,500 mappings each merging a 64-key anchor exceeds the cap and throws RangeError("Out of memory"). The alias_expansion_budget added in #31495 already bounds this case: every `*anchor` in a `<<: *anchor` is charged its full subtree (1 + 2*props nodes) before merge() runs, so the total number of properties materialized through merge keys is bounded by MAX_ALIAS_EXPANSION/2. Inline merge values (`<<: {...}`) are bounded by the input size. The separate merge budget is redundant and too tight; the Zig reference implementation has no such limit. Replace the hardening test that asserted the old cap with one that verifies the 380 KB / 64x16,500 document parses correctly. The exponential-expansion guard (alias_expansion_budget) is unchanged and its test still passes. --- src/parsers/yaml.rs | 32 +++++++++---------------------- test/js/bun/yaml/yaml.test.ts | 36 ++++++++++++++++++++++------------- 2 files changed, 32 insertions(+), 36 deletions(-) diff --git a/src/parsers/yaml.rs b/src/parsers/yaml.rs index 2a1245dfbf9..e86081bfe21 100644 --- a/src/parsers/yaml.rs +++ b/src/parsers/yaml.rs @@ -2354,7 +2354,6 @@ pub struct Parser<'i, Enc: Encoding> { pub stack_check: StackCheck, - pub merge_props_budget: usize, pub alias_expansion_budget: usize, } @@ -2390,7 +2389,6 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { tag_handles: StringHashMap::default(), whitespace_buf: Vec::new(), stack_check: StackCheck::init(), - merge_props_budget: MappingProps::MAX_MERGED_PROPERTIES, alias_expansion_budget: Self::MAX_ALIAS_EXPANSION, } } @@ -2781,7 +2779,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { Expr::init(E::Null {}, self.token.start.loc()) }; let mut props = MappingProps::init(); - props.append_maybe_merge(key, value, &mut self.merge_props_budget)?; + props.append_maybe_merge(key, value)?; Expr::init( E::Object { properties: props.move_list(), @@ -2914,7 +2912,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { current_mapping_indent: Some(self.token.indent), ..Default::default() })?; - props.append_maybe_merge(key, value, &mut self.merge_props_budget)?; + props.append_maybe_merge(key, value)?; } // [140] ns-s-flow-map-entries: after an entry, only `,` or `}`. @@ -3104,7 +3102,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { _ => Expr::init(E::Null {}, mapping_value_start.loc()), }; - props.append_maybe_merge(first_key, value, &mut self.merge_props_budget)?; + props.append_maybe_merge(first_key, value)?; } if self.context.get() == Context::FlowIn { @@ -3236,7 +3234,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { } }; - props.append_maybe_merge(key, value, &mut self.merge_props_budget)?; + props.append_maybe_merge(key, value)?; } Ok(Expr::init( @@ -3270,8 +3268,6 @@ pub struct MappingProps { } impl MappingProps { - pub const MAX_MERGED_PROPERTIES: usize = 1024 * 1024; - pub fn init() -> Self { Self { list: bun_alloc::AstAlloc::vec(), @@ -3285,12 +3281,8 @@ impl MappingProps { Ok(()) } - pub fn merge( - &mut self, - merge_props: &[G::Property], - budget: &mut usize, - ) -> Result<(), AllocError> { - self.list.reserve(merge_props.len().min(*budget)); + pub fn merge(&mut self, merge_props: &[G::Property]) -> Result<(), AllocError> { + self.list.reserve(merge_props.len()); while self.merge_indexed < self.list.len() { let idx = self.merge_indexed; @@ -3314,7 +3306,6 @@ impl MappingProps { } } } - *budget = budget.checked_sub(1).ok_or(AllocError)?; // `G::Property` is not `Clone`; reconstruct from its `Copy` fields. self.list.push(G::Property { key: merge_prop.key, @@ -3333,12 +3324,7 @@ impl MappingProps { Ok(()) } - pub fn append_maybe_merge( - &mut self, - key: Expr, - value: Expr, - budget: &mut usize, - ) -> Result<(), AllocError> { + pub fn append_maybe_merge(&mut self, key: Expr, value: Expr) -> Result<(), AllocError> { let is_merge_key = match &key.data { ast::ExprData::EString(key_str) => key_str.eql_comptime(b"<<"), _ => false, @@ -3354,14 +3340,14 @@ impl MappingProps { } match &value.data { - ast::ExprData::EObject(value_obj) => self.merge(value_obj.properties.slice(), budget), + ast::ExprData::EObject(value_obj) => self.merge(value_obj.properties.slice()), ast::ExprData::EArray(value_arr) => { for item in value_arr.items.slice() { let item_obj = match &item.data { ast::ExprData::EObject(obj) => obj, _ => continue, }; - self.merge(item_obj.properties.slice(), budget)?; + self.merge(item_obj.properties.slice())?; } Ok(()) } diff --git a/test/js/bun/yaml/yaml.test.ts b/test/js/bun/yaml/yaml.test.ts index a7242cdb2f7..5071d881c37 100644 --- a/test/js/bun/yaml/yaml.test.ts +++ b/test/js/bun/yaml/yaml.test.ts @@ -4473,7 +4473,7 @@ test("merging the same large anchor many times completes quickly", () => { expect(elapsed).toBeLessThan(isDebug || isASAN ? 15_000 : 4_000); }, 30_000); -test("limits how many properties merge keys can materialize from a small document", () => { +test("merge keys across many mappings are bounded only by the alias-expansion budget", () => { // A normal merge-key document still resolves. const small = YAML.parse("base: &base\n x: 1\n y: 2\nchild:\n <<: *base\n z: 3\n") as { base: Record; @@ -4481,26 +4481,36 @@ test("limits how many properties merge keys can materialize from a small documen }; expect(small.child).toEqual({ x: 1, y: 2, z: 3 }); - // One anchor with `keyCount` properties merged into `mergeCount` separate - // mappings would materialize keyCount * mergeCount (~1.2 million) property - // entries from a ~30 KB document. The parser caps the total number of - // properties materialized through merge keys and reports an error instead - // of allocating memory proportional to the product. - const keyCount = 2048; - const mergeCount = 600; + // One 64-key anchor merged into 16,500 separate mappings materializes just + // over a million properties from a ~380 KB document. Every `*base` reference + // is already charged against the alias-expansion budget (16M nodes), so the + // parser must accept this document rather than imposing a separate + // per-stream cap on merged properties. + const keyCount = 64; + const mergeCount = 16_500; - const lines: string[] = ["a: &a"]; + const lines: string[] = ["base: &base"]; for (let i = 0; i < keyCount; i++) { lines.push(` k${i}: ${i}`); } + lines.push("out:"); for (let i = 0; i < mergeCount; i++) { - lines.push(`m${i}:`); - lines.push(" <<: *a"); + lines.push(` m${i}:`); + lines.push(" <<: *base"); } const input = lines.join("\n"); - expect(() => YAML.parse(input)).toThrow(); -}, 30_000); + const parsed = YAML.parse(input) as { + base: Record; + out: Record>; + }; + + expect(Object.keys(parsed.base)).toHaveLength(keyCount); + expect(Object.keys(parsed.out)).toHaveLength(mergeCount); + expect(parsed.out.m0).toEqual(parsed.base); + expect(parsed.out[`m${mergeCount - 1}`]).toEqual(parsed.base); + expect(parsed.out.m0[`k${keyCount - 1}`]).toBe(keyCount - 1); +}, 120_000); test("bounds alias expansion for parsed and imported YAML documents", async () => { // A document with a few levels of anchors, where each level is a sequence of From 33ed6619b6a2cdeeae05b77732def3d043c7430d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 16 Jun 2026 09:20:33 +0000 Subject: [PATCH 2/4] yaml: charge merge-key copies against the alias-expansion budget Nested inline merge wrappers (`{<<: {<<: ... {<<: *big}}}`) resolve the innermost `*big` once but re-materialize its properties at every nesting level, so depth * keyCount properties are allocated while alias_expansion_budget is only charged once. Charge each merge() call's input slice against the same budget so nested merges are bounded the same way direct alias references are, and surface the limit as ParseError::ExcessiveAliasing rather than OutOfMemory. The 64 x 16,500 reproduction consumes ~3.2M of the 16M budget and still parses; the nested-wrapper attack is now rejected. --- src/parsers/yaml.rs | 35 +++++++++++++++++++++++------- test/js/bun/yaml/yaml.test.ts | 40 +++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/parsers/yaml.rs b/src/parsers/yaml.rs index e86081bfe21..55cf73ff39c 100644 --- a/src/parsers/yaml.rs +++ b/src/parsers/yaml.rs @@ -2779,7 +2779,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { Expr::init(E::Null {}, self.token.start.loc()) }; let mut props = MappingProps::init(); - props.append_maybe_merge(key, value)?; + props.append_maybe_merge(key, value, &mut self.alias_expansion_budget)?; Expr::init( E::Object { properties: props.move_list(), @@ -2912,7 +2912,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { current_mapping_indent: Some(self.token.indent), ..Default::default() })?; - props.append_maybe_merge(key, value)?; + props.append_maybe_merge(key, value, &mut self.alias_expansion_budget)?; } // [140] ns-s-flow-map-entries: after an entry, only `,` or `}`. @@ -3102,7 +3102,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { _ => Expr::init(E::Null {}, mapping_value_start.loc()), }; - props.append_maybe_merge(first_key, value)?; + props.append_maybe_merge(first_key, value, &mut self.alias_expansion_budget)?; } if self.context.get() == Context::FlowIn { @@ -3234,7 +3234,7 @@ impl<'i, Enc: Encoding> Parser<'i, Enc> { } }; - props.append_maybe_merge(key, value)?; + props.append_maybe_merge(key, value, &mut self.alias_expansion_budget)?; } Ok(Expr::init( @@ -3281,7 +3281,21 @@ impl MappingProps { Ok(()) } - pub fn merge(&mut self, merge_props: &[G::Property]) -> Result<(), AllocError> { + pub fn merge( + &mut self, + merge_props: &[G::Property], + budget: &mut usize, + ) -> Result<(), ParseError> { + // The `merge_props` slice may have been produced by an inner `<<:` + // that already expanded an alias, so a chain of inline wrappers + // (`{<<: {<<: ... {<<: *big}}}`) re-materializes the same properties + // at every level without any further alias resolution. Charge each + // copy against the alias-expansion budget so nested merges are + // bounded the same way direct `*alias` references are. + *budget = budget + .checked_sub(merge_props.len()) + .ok_or(ParseError::ExcessiveAliasing)?; + self.list.reserve(merge_props.len()); while self.merge_indexed < self.list.len() { @@ -3324,7 +3338,12 @@ impl MappingProps { Ok(()) } - pub fn append_maybe_merge(&mut self, key: Expr, value: Expr) -> Result<(), AllocError> { + pub fn append_maybe_merge( + &mut self, + key: Expr, + value: Expr, + budget: &mut usize, + ) -> Result<(), ParseError> { let is_merge_key = match &key.data { ast::ExprData::EString(key_str) => key_str.eql_comptime(b"<<"), _ => false, @@ -3340,14 +3359,14 @@ impl MappingProps { } match &value.data { - ast::ExprData::EObject(value_obj) => self.merge(value_obj.properties.slice()), + ast::ExprData::EObject(value_obj) => self.merge(value_obj.properties.slice(), budget), ast::ExprData::EArray(value_arr) => { for item in value_arr.items.slice() { let item_obj = match &item.data { ast::ExprData::EObject(obj) => obj, _ => continue, }; - self.merge(item_obj.properties.slice())?; + self.merge(item_obj.properties.slice(), budget)?; } Ok(()) } diff --git a/test/js/bun/yaml/yaml.test.ts b/test/js/bun/yaml/yaml.test.ts index 5071d881c37..14c75be509e 100644 --- a/test/js/bun/yaml/yaml.test.ts +++ b/test/js/bun/yaml/yaml.test.ts @@ -4512,6 +4512,46 @@ test("merge keys across many mappings are bounded only by the alias-expansion bu expect(parsed.out.m0[`k${keyCount - 1}`]).toBe(keyCount - 1); }, 120_000); +test("bounds merge-key materialization through nested inline wrappers", () => { + // `{<<: {<<: ... {<<: *big}}}` resolves `*big` once but re-materializes + // every property at each nesting level, so depth * keyCount properties + // are allocated from an input that is linear in depth + keyCount. Each + // copy must be charged against the alias-expansion budget. + function wrap(keyCount: number, depth: number) { + const keys: string[] = []; + for (let i = 0; i < keyCount; i++) keys.push(`k${i}: ${i}`); + let out = "out: "; + for (let i = 0; i < depth; i++) out += "{<<: "; + out += "*big"; + for (let i = 0; i < depth; i++) out += "}"; + return [`big: &big {${keys.join(", ")}}`, out]; + } + + // A reasonable nesting depth still parses and produces the merged result. + const ok = YAML.parse(wrap(100, 50).join("\n") + "\n") as { + big: Record; + out: Record; + }; + expect(Object.keys(ok.out)).toHaveLength(100); + expect(ok.out).toEqual(ok.big); + + // Pre-consume the alias-expansion budget down to a small remainder, then + // show that nested-merge copies are charged against the same budget: 200 + // inline wrappers around a 100-key anchor push it over the limit and are + // rejected rather than silently materializing depth * keyCount properties. + const width = 30; + const fan = (ref: string) => `[${new Array(width).fill(ref).join(", ")}]`; + const bomb = [ + `a: &a ${fan("0")}`, + `b: &b ${fan("*a")}`, + `c: &c ${fan("*b")}`, + `d: &d ${fan("*c")}`, + `pad: [${new Array(18).fill("*d").concat(new Array(29).fill("*c")).join(", ")}]`, + ]; + const payload = bomb.concat(wrap(100, 200)).join("\n") + "\n"; + expect(() => YAML.parse(payload)).toThrow(/[Ee]xcessive aliasing/); +}, 30_000); + test("bounds alias expansion for parsed and imported YAML documents", async () => { // A document with a few levels of anchors, where each level is a sequence of // aliases to the previous one, expands to width^depth nodes even though the From 4380606e5ab0b39c93227c96cd6b158f7caf58cf Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 16 Jun 2026 09:50:30 +0000 Subject: [PATCH 3/4] test(yaml): assert nested-merge bomb precondition stays under budget The bomb pre-consumes ~99.9% of MAX_ALIAS_EXPANSION via tuned multipliers. If node-counting or the constant ever shifts, the bomb alone would throw ExcessiveAliasing and the final assertion would pass for the wrong reason. Append an unresolved-alias probe that only reaches the Unresolved alias error if the parser got past the bomb without exhausting the budget (and avoids materializing ~16M JS values on the success path). --- test/js/bun/yaml/yaml.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/js/bun/yaml/yaml.test.ts b/test/js/bun/yaml/yaml.test.ts index 14c75be509e..c2fca3b9e7a 100644 --- a/test/js/bun/yaml/yaml.test.ts +++ b/test/js/bun/yaml/yaml.test.ts @@ -4539,6 +4539,11 @@ test("bounds merge-key materialization through nested inline wrappers", () => { // show that nested-merge copies are charged against the same budget: 200 // inline wrappers around a 100-key anchor push it over the limit and are // rejected rather than silently materializing depth * keyCount properties. + // + // Each `*d` reference walks 837,931 nodes and each `*c` walks 27,931, so + // building a..d plus `pad: [*d x 18, *c x 29]` charges ~16,759,547 of the + // 16,777,216 budget (MAX_ALIAS_EXPANSION), leaving ~17,669 for the nested + // merges. const width = 30; const fan = (ref: string) => `[${new Array(width).fill(ref).join(", ")}]`; const bomb = [ @@ -4548,6 +4553,13 @@ test("bounds merge-key materialization through nested inline wrappers", () => { `d: &d ${fan("*c")}`, `pad: [${new Array(18).fill("*d").concat(new Array(29).fill("*c")).join(", ")}]`, ]; + // Precondition: the bomb on its own must stay under the budget; if it did + // not, the final assertion could pass because `pad:` threw, not because + // nested merges are charged. Appending an unresolved alias throws + // "Unresolved alias" only if the parser got past `pad:` without exhausting + // the budget (and avoids materializing the bomb to JS on success). + expect(() => YAML.parse(bomb.join("\n") + "\nprobe: *nope\n")).toThrow(/Unresolved alias/); + const payload = bomb.concat(wrap(100, 200)).join("\n") + "\n"; expect(() => YAML.parse(payload)).toThrow(/[Ee]xcessive aliasing/); }, 30_000); From 186daa603ff526622dce7be004a24bbb31f279b9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 16 Jun 2026 10:33:32 +0000 Subject: [PATCH 4/4] ci: retrigger