fix: bound parser paths reachable from a crafted document

[sondt99]

Five resource-exhaustion paths are reachable from untrusted input. One is a hard crash: a ~1.6 MB .ppt aborts the process with a stack overflow.

Each is bounded by a constant in package::limits now, so it returns ResourceLimit like every other attack shape.

ppt — stack overflow (SIGSEGV)

Extractor::walk documents itself as "Iterative container walk over an explicit stack with fixed depth and record-count bounds" and enforces MAX_RECORD_DEPTH against stack.len(). But the recovery-mode NotesContainer branch called self.walk(body) again. The re-entrant call starts a fresh local stack, so the depth bound resets to 1 at every level and never fires — nesting ends up capped only by MAX_RECORDS (16M), orders of magnitude past what the real stack survives.

recovering is attacker-controlled: any unusable persist directory sets it.

Reproduced on 261fc25:

test formats::ppt::tests::nested_notes_in_recovery_hit_the_depth_bound ... FAILED
thread '...::deeply_nested_notes_do_not_overflow_the_stack' has overflowed its stack
fatal runtime error: stack overflow, aborting

The descent is a stack push carrying an end-of-segment flag now, so the end_segment / current_is_notes ordering is byte-for-byte what the recursive version did.

rtf — stack overflow

Math zones build their Element tree through MathState::open_group, not parse_xml, so MAX_XML_DEPTH never applied to it. close_groups nests each group into its parent, making tree depth equal brace depth, and the OMML serializer (walk_children/walk_elem) recurses once per level — as does dropping the tree. Capped at construction, since a guard in the serializer would not save the recursive Drop.

Separately the group stack retained a CharState per { with no cap: ~10 MB of { → ~1 GB RSS. Now MAX_RTF_GROUP_DEPTH.

doc — OOM and allocator abort

FKP page numbers in the PLC are masked to 22 bits and nothing stops every entry pointing at the same page, so the entry count did not bound the run list — a few MB of table stream expanded into tens of GB. Bounded by MAX_DOC_RUNS; formatting degrades past it, text extraction is unaffected.

parse_plf_lfo also sized a Vec straight from a raw u32 header field, so 0xFFFFFFFF asked the allocator for ~68 GB and hit handle_alloc_error → abort(). Clamped to what the buffer can hold; the existing loop already bounded the real count.

sheet — OOM

formatCode has no length bound anywhere and parse_section materializes several vectors per character, so a compressed archive entry amplified into GBs without a worksheet cell being involved (Styles::read parses every cellXfs/xf unconditionally). Capped by MAX_NUMBER_FORMAT_BYTES, falling back to General.

Also

#![forbid(unsafe_code)] at the crate root. The crate already had zero unsafe — this makes the property enforced rather than incidental. (Scoped to this crate; the parsing dependencies are unaffected.)

Tests

Four regression tests, two of which fail on the parent commit — the .ppt one by aborting the test binary.

cargo fmt --check clean · cargo clippy --all-targets -- -D warnings exit 0 · 300 tests pass · workspace incl. Python and WASM bindings compiles.

Happy to split this into per-format PRs, or to move the crash to a private advisory instead if you'd rather it not sit in a public diff — there is no SECURITY.md, and since this is a DoS in safe Rust rather than memory corruption I defaulted to the normal PR flow.

---

Summary by cubic

Fixes five resource-exhaustion paths reachable from crafted documents, including a ~1.6 MB .ppt that aborts the process with a stack overflow. Each path now returns ResourceLimit instead of crashing or exhausting memory, matching how other attack shapes are handled.

Bug Fixes

• The recovery-mode notes container in ppt now descends via the shared bounded stack instead of re-entering walk, which reset the depth bound to 1 at every level and left nesting capped only by the 16M record limit.
• RTF math-zone nesting is capped at construction, since the recursive Drop would defeat a serializer-side guard; the group stack also gets its own cap to bound retained character-state snapshots.
• .doc FKP formatting runs are bounded, and parse_plf_lfo clamps a raw header count to the buffer length instead of reserving tens of gigabytes from a value like 0xFFFFFFFF.
• formatCode longer than 4096 bytes falls back to General rather than amplifying a compressed archive entry into gigabytes.
• #![forbid(unsafe_code)] makes the crate's existing safe-only property enforced rather than incidental.

^{Written for commit f8c87cc. Summary will update on new commits.}

[cubic-dev-ai]

3 issues found across 7 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/package/limits.rs">

<violation number="1" location="src/package/limits.rs:47">
P3: The module header states "crossing one returns ConvertError::ResourceLimit, always." The three new caps violate that invariant: MAX_DOC_RUNS degrades formatting by ignoring later runs, and MAX_NUMBER_FORMAT_BYTES falls back to a General format, and neither returns ResourceLimit (only MAX_RTF_GROUP_DEPTH does). Update the header (or the new comments) so the documented contract accurately reflects that some of these boundaries degrade gracefully rather than returning ResourceLimit, so callers don't rely on ResourceLimit for the two that don't raise it.</violation>
</file>

<file name="src/formats/ppt/mod.rs">

<violation number="1" location="src/formats/ppt/mod.rs:663">
P2: The 200,000-level regression test copies the entire fixture on every iteration, writing about 160 GB for a final 1.6 MB input. Construct the final buffer once and write each header in place so this test does not make CI unnecessarily slow.</violation>
</file>

<file name="src/formats/rtf/mod.rs">

<violation number="1" location="src/formats/rtf/mod.rs:1369">
P2: Because `\*` suppresses the following destination, this test never creates a `MathState` or exercises the new XML-depth bound. Use the unstarred `\mmath` destination so the regression test actually covers `open_group`.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The 200,000-level regression test copies the entire fixture on every iteration, writing about 160 GB for a final 1.6 MB input. Construct the final buffer once and write each header in place so this test does not make CI unnecessarily slow.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/formats/ppt/mod.rs, line 663:

<comment>The 200,000-level regression test copies the entire fixture on every iteration, writing about 160 GB for a final 1.6 MB input. Construct the final buffer once and write each header in place so this test does not make CI unnecessarily slow.</comment>

<file context>
@@ -634,3 +651,55 @@ impl Extractor {
+    /// `depth` NotesContainers (0x03F0), each holding the next. No NotesAtom
+    /// child, so none of them reads as the notes master.
+    fn nested_notes(depth: usize) -> Vec<u8> {
+        let mut buf: Vec<u8> = Vec::new();
+        for _ in 0..depth {
+            let mut outer = Vec::with_capacity(buf.len() + 8);
</file context>

Suggested change

	let mut buf: Vec<u8> = Vec::new();
	for _ in 0..depth {
	let mut outer = Vec::with_capacity(buf.len() + 8);
	outer.extend_from_slice(&0x000Fu16.to_le_bytes()); // container
	outer.extend_from_slice(&0x03F0u16.to_le_bytes()); // NotesContainer
	outer.extend_from_slice(&(buf.len() as u32).to_le_bytes());
	outer.extend_from_slice(&buf);
	buf = outer;
	}
	let mut buf = vec![0u8; depth * 8];
	for i in 0..depth {
	let off = i * 8;
	buf[off..off + 2].copy_from_slice(&0x000Fu16.to_le_bytes());
	buf[off + 2..off + 4].copy_from_slice(&0x03F0u16.to_le_bytes());
	let len = ((depth - i - 1) * 8) as u32;
	buf[off + 4..off + 8].copy_from_slice(&len.to_le_bytes());
	}

[cubic-dev-ai]

P2: Because \* suppresses the following destination, this test never creates a MathState or exercises the new XML-depth bound. Use the unstarred \mmath destination so the regression test actually covers open_group.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/formats/rtf/mod.rs, line 1369:

<comment>Because `\*` suppresses the following destination, this test never creates a `MathState` or exercises the new XML-depth bound. Use the unstarred `\mmath` destination so the regression test actually covers `open_group`.</comment>

<file context>
@@ -1329,6 +1347,36 @@ impl<'a> Parser<'a> {
+    /// dropping the tree.
+    #[test]
+    fn deep_math_nesting_stays_within_the_xml_depth_bound() {
+        let mut src = String::from(r"{\rtf1\ansi {\*\mmath ");
+        // Stay under the group bound so this exercises the math cap alone.
+        let groups = limits::MAX_XML_DEPTH * 2;
</file context>

Suggested change

	let mut src = String::from(r"{\rtf1\ansi {\*\mmath ");
	let mut src = String::from(r"{\rtf1\ansi {\mmath ");

[cubic-dev-ai]

P3: The module header states "crossing one returns ConvertError::ResourceLimit, always." The three new caps violate that invariant: MAX_DOC_RUNS degrades formatting by ignoring later runs, and MAX_NUMBER_FORMAT_BYTES falls back to a General format, and neither returns ResourceLimit (only MAX_RTF_GROUP_DEPTH does). Update the header (or the new comments) so the documented contract accurately reflects that some of these boundaries degrade gracefully rather than returning ResourceLimit, so callers don't rely on ResourceLimit for the two that don't raise it.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/package/limits.rs, line 47:

<comment>The module header states "crossing one returns ConvertError::ResourceLimit, always." The three new caps violate that invariant: MAX_DOC_RUNS degrades formatting by ignoring later runs, and MAX_NUMBER_FORMAT_BYTES falls back to a General format, and neither returns ResourceLimit (only MAX_RTF_GROUP_DEPTH does). Update the header (or the new comments) so the documented contract accurately reflects that some of these boundaries degrade gracefully rather than returning ResourceLimit, so callers don't rely on ResourceLimit for the two that don't raise it.</comment>

<file context>
@@ -40,5 +40,23 @@ pub const MAX_ASSET_TOTAL_BYTES: usize = 128 * 1024 * 1024;
+/// page numbers in the PLC are attacker-controlled and may all alias a single
+/// page, so the entry count on its own does not bound how many runs they
+/// yield. Past the cap formatting degrades; text extraction is unaffected.
+pub const MAX_DOC_RUNS: usize = 1_000_000;
+
+/// Maximum RTF group nesting depth. Real documents nest a few dozen groups;
</file context>