Skip to content

fix(doc): bound the LFO reservation by the records the table holds - #195

Open
sanghnkim-max wants to merge 1 commit into
firecrawl:mainfrom
sanghnkim-max:lfo-count-bound
Open

sanghnkim-max wants to merge 1 commit into
firecrawl:mainfrom
sanghnkim-max:lfo-count-bound

Conversation

@sanghnkim-max

@sanghnkim-max sanghnkim-max commented Oct 16, 2026 •

Copy link
Copy Markdown

parse_plf_lfo (src/formats/doc/lists.rs) passes the PlfLfo's u32 count straight to Vec::with_capacity. A malformed .doc whose LFO table declares 0xFFFF_FFFF records but holds none asks for about 64 GB (16-byte (u32, usize) elements) before the loop finds that the records are missing. Where the allocator refuses, Rust aborts the process instead of panicking, so a caller can't recover. The loop already stops at the first missing record, so the reservation was the only unbounded step.

  • lfo_capacity clamps the reservation to (plf.len() - 4) / LFO_SIZE, the number of records that fit in the table.
  • The regression test asserts on that capacity rather than by allocating. On an overcommitting allocator (macOS, and Linux by default) the unbounded 64 GB reservation "succeeds", so a test that only parsed the table would pass even without the fix. I confirmed that this test fails with the bound removed and passes with it.

I checked the other with_capacity calls under src/. Each is bounded by a u16/u8 count or by the input's length; this was the only one taking a full u32 from the file.

cargo test --workspace (297 passed), cargo clippy --all-targets -- -D warnings and cargo fmt --check are all clean.

🤖 Generated with Claude Code


Summary by cubic

Fixes a parser DoS where a malformed .doc with an oversized LFO count could abort the process by requesting ~64 GB of memory up front.

  • lfo_capacity clamps the Vec reservation to the records the table can actually hold, matching the loop's existing stop-at-first-missing-record behavior.
  • Adds a regression test asserting on the reserved capacity, since overcommitting allocators would hide the bug in an allocation-based test.

Written for commit 0692341. Summary will update on new commits.

View guided diff

parse_plf_lfo reserved Vec capacity straight from the PlfLfo's u32 count.
A short table declaring 0xFFFF_FFFF asked for ~64 GB up front, which
aborts the process wherever the allocator refuses it; the loop below
already stops at the first missing record, so the reservation was the
only unbounded step.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

View guided diff | Re-trigger cubic

This branch has not been deployed

No deployments
Sign up for free to 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.

1 participant