Skip to content

Bound the legacy .xls container reader by the file's own size - #7

Merged
almasumdev merged 1 commit into
almasumdev:mainfrom
cdeil:cfb-bounds
Oct 16, 2026
Merged

almasumdev merged 1 commit into
almasumdev:mainfrom
cdeil:cfb-bounds

Conversation

@cdeil

@cdeil cdeil commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Problem. The OLE2 (MS-CFB) reader behind Excel.decodeBytes for binary
.xls input trusts three 32-bit header fields the file controls: the number
of DIFAT sectors, the number of mini FAT sectors, and a stream's declared size.
A 1,024-byte file whose only DIFAT sector names itself as the next one, with a
declared count of 50,000, allocates about 95 MB before being rejected for
having no Workbook stream; counts are 32-bit, so that is a floor, not a
ceiling. A FAT entry that points a mini FAT sector at itself does the same
through _readMiniFat. Since the reader is chosen by magic bytes, a file named
.xlsx reaches it too.

Measured on the audited commit (macOS arm64, process RSS delta, illustrative):

Vector Declared count Input RSS growth
cyclic DIFAT 50,000 1,024 B ~95 MB
cyclic mini FAT chain 50,000 1,536 B ~96 MB

Fix. Every chain and count is bounded by the sectors the file physically
holds. A DIFAT sector or FAT sector listed twice is rejected as corrupt before
it is read again; _chain uses that physical bound instead of _fat.length
(which an inflated FAT could stretch); the mini FAT is read as a checked chain;
a stream's declared size must fit its sector chain, and a mini stream its
container, before a buffer of that size is created; the mini chain walk has a
step bound. Everything still throws ExcelFormatException, as the class
promises for corrupt structures. Sound files read exactly as before.

Validation. New test/xls_compound_test.dart: cyclic DIFAT, cyclic mini
FAT, duplicate FAT sector, oversized declared stream size (4 GB in a 7 KB
file), and a sound regular-sector workbook. The first three fail on the current
reader and pass with the change; the oversized-size case was only run after
the change. The existing .xls suites (xls_reader_test, xls_formula_test,
real BIFF8 fixtures) pass unchanged; full suite and dart analyze clean. The
bounded reproductions now reject in 0–1 ms with negligible observed RSS growth.
A comparison against 52 external .xls fixtures from xlrd and Apache POI
decodes the same 48 files, rejects the same four files, and produces identical
compared cell snapshots before and after the change.

🤖 Generated with Claude Code

The OLE2 reader trusted three 32-bit header fields a file controls: the
number of DIFAT sectors, the number of mini FAT sectors, and a stream's
declared size. A 1 KB file whose single DIFAT sector names itself as the
next one, with a count of 50,000, grew the process by about 95 MB before
being rejected for having no Workbook stream, and a self-referencing FAT
entry under the mini FAT did the same. Since decodeBytes picks this
reader by magic bytes, an .xlsx-named upload could reach it.

Every chain is now bounded by the sectors the file physically holds: a
DIFAT or FAT sector listed twice, or a chain longer than the file, is
rejected as corrupt before it is read again, the mini FAT is walked as
a checked chain, and a stream's size must fit its chain before a buffer
of that size is made for it. Sound files are read as before.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@almasumdev
almasumdev merged commit 9536f17 into almasumdev:main Oct 16, 2026
8 checks passed
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.

2 participants