Skip to content

fix(expand): bound the total nodes produced by for-each - #261

Merged
LeadcodeDev merged 1 commit into
chantier/audit-2026-09from
fix/expand-node-budget
Sep 22, 2026
Merged

LeadcodeDev merged 1 commit into
chantier/audit-2026-09from
fix/expand-node-budget

Conversation

@LeadcodeDev

Copy link
Copy Markdown
Owner

Severity Low, category security. Location: crates/rustmotion-core/src/expand.rs:157

Impact

MAX_EXPANSION_DEPTH bounds how deeply directives may nest, not how many nodes they produce, and nesting one for-each inside another's template is explicitly supported (use_template_can_contain_a_nested_for_each, expand.rs:1005). Each nesting level costs exactly +1 depth (resolve_entry recurses with depth + 1 per produced node, while walk_children forwards depth unchanged), so up to 64 levels are legal and the output is the product of the array lengths. Four nested levels of 50 elements is 6.25M nodes from a file under 1 KB; eight levels of 10 is 10^8. Each iteration does a full Value deep-clone of the template plus a substitute walk, so the process hangs and OOMs during expand_directives — before validation, before any render, and with no diagnostic. An LLM-authored or third-party scenario reaches this through the documented public syntax; --no-validate is not needed.

Fix

Thread a mutable node/iteration budget through resolve_entry (e.g. a &mut u64 counter decremented per produced node, ceiling in the low millions) and fail with a named error when it is exhausted, exactly as ExpansionDepthExceeded does for depth. Cap items.len() per directive as a cheap first line of defence.

Evidence the audit read

/// Defense-in-depth ceiling on nested `use`/`for-each` expansion. ...
const MAX_EXPANSION_DEPTH: u32 = 64;
---
fn resolve_entry(... depth: u32) -> Result<Vec<Value>> {
    if depth > MAX_EXPANSION_DEPTH { return Err(...ExpansionDepthExceeded...); }
---
    let mut out = Vec::with_capacity(items.len());
    for (idx, element) in items.into_iter().enumerate() {
        ...
        let mut node = directive.template.clone();
        substitute(&mut node, &bindings, file_label)?;
        out.push(node);

Stacked on fix/media-protocol-allowlist, which carries the previous finding of this workstream. GitHub shows only this finding's diff; merge in order.

Part of the September 2026 audit remediation chantier. Refs #220 (RM-43).

@LeadcodeDev LeadcodeDev added the bug Something isn't working label Sep 21, 2026
@LeadcodeDev LeadcodeDev self-assigned this Sep 21, 2026
@LeadcodeDev
LeadcodeDev force-pushed the fix/media-protocol-allowlist branch from 3e4b39f to e9965fc Compare September 22, 2026 06:11
@LeadcodeDev
LeadcodeDev force-pushed the fix/expand-node-budget branch from 62393d5 to 4dc047f Compare September 22, 2026 06:11
@LeadcodeDev
LeadcodeDev force-pushed the fix/media-protocol-allowlist branch from e9965fc to edd1852 Compare September 22, 2026 08:36
@LeadcodeDev
LeadcodeDev force-pushed the fix/expand-node-budget branch from 4dc047f to 9daff5f Compare September 22, 2026 08:36
@LeadcodeDev
LeadcodeDev force-pushed the fix/media-protocol-allowlist branch from edd1852 to 50f6268 Compare September 22, 2026 08:45
@LeadcodeDev
LeadcodeDev force-pushed the fix/expand-node-budget branch from 9daff5f to 226b364 Compare September 22, 2026 08:46
@LeadcodeDev
LeadcodeDev changed the base branch from fix/media-protocol-allowlist to chantier/audit-2026-09 September 22, 2026 08:54
@LeadcodeDev
LeadcodeDev force-pushed the fix/expand-node-budget branch from 226b364 to 690c00b Compare September 22, 2026 09:02
`MAX_EXPANSION_DEPTH` bounds how deeply directives may nest, not how many nodes they produce, and nesting one `for-each` inside another's `template` is explicitly supported (`use_template_can_contain_a_nested_for_each`, expand.rs:1005). Each nesting level costs exactly +1 depth (resolve_entry recurses with `depth + 1` per produced node, while `walk_children` forwards `depth` unchanged), so up to 64 levels are legal and the output is the product of the array lengths. Four nested levels of 50 elements is 6.25M nodes from a file under 1 KB; eight levels of 10 is 10^8. Each iteration does a full `Value` deep-clone of the template plus a `substitute` walk, so the process hangs and OOMs during `expand_directives` — before validation, before any render, and with no diagnostic. An LLM-authored or third-party scenario reaches this through the documented public syntax; `--no-validate` is not needed.

Refs #220
@LeadcodeDev
LeadcodeDev force-pushed the fix/expand-node-budget branch from 690c00b to 958bbdb Compare September 22, 2026 09:06
@LeadcodeDev
LeadcodeDev merged commit 0f3dc12 into chantier/audit-2026-09 Sep 22, 2026
LeadcodeDev added a commit that referenced this pull request Sep 22, 2026
`MAX_EXPANSION_DEPTH` bounds how deeply directives may nest, not how many nodes they produce, and nesting one `for-each` inside another's `template` is explicitly supported (`use_template_can_contain_a_nested_for_each`, expand.rs:1005). Each nesting level costs exactly +1 depth (resolve_entry recurses with `depth + 1` per produced node, while `walk_children` forwards `depth` unchanged), so up to 64 levels are legal and the output is the product of the array lengths. Four nested levels of 50 elements is 6.25M nodes from a file under 1 KB; eight levels of 10 is 10^8. Each iteration does a full `Value` deep-clone of the template plus a `substitute` walk, so the process hangs and OOMs during `expand_directives` — before validation, before any render, and with no diagnostic. An LLM-authored or third-party scenario reaches this through the documented public syntax; `--no-validate` is not needed.

Refs #220
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant