read_shared_strings reserves Vec<String> capacity from unbounded attacker-controlled uniqueCount

[devsamhan]

Summary

Xlsx::read_shared_strings (src/xlsx/mod.rs) reserves Vec<String> capacity directly from the attacker-controlled uniqueCount attribute in sharedStrings.xml, with no upper bound.

Affected version observed locally: calamine 0.36.0. I have not verified whether this is already fixed on main; this report is based on local source inspection of calamine-0.36.0 only (no CI/test-suite check was performed against a newer revision).

Affected code path

In src/xlsx/mod.rs, read_shared_strings parses the <sst uniqueCount="N"> root attribute and calls self.strings.reserve(n):

if let Some(count) = e.raw_attr(b"uniqueCount")? {
    if let Ok(n) = atoi_simd::parse::<usize, true, false>(count) {
        self.strings.reserve(n);
    }
}

This runs unconditionally inside Xlsx::new()'s constructor, before the caller regains control — there is no point between calling open_workbook/open_workbook_auto and getting a Result back where the caller's own code executes.

Contrast with worksheet_range_ref's structurally identical <dimension>-driven reservation, which the same file already guards:

let len = cell_reader.dimensions().len();
let mut cells = Vec::new();
if len < 100_000 {
    cells.reserve(len as usize);
}

read_shared_strings's uniqueCount-driven reservation has no equivalent guard.

Impact

A ~1 KB, otherwise perfectly ordinary .xlsx (normal ZIP central directory, no unusual compression ratio, no oversized declared/actual size anywhere) can declare uniqueCount="4000000000" in sharedStrings.xml with zero <si> children, driving a single Vec::reserve request for ~96 GB (4,000,000,000 × 24 bytes/String header on a 64-bit target) before the caller ever regains control.

Because this uses the non-fallible .reserve() (not try_reserve), an allocation failure invokes handle_alloc_error, whose default behavior is to abort the process — not a catchable Result. A calling application's ordinary Result-based error handling around open_workbook/open_workbook_auto cannot intercept this; the effect is a crash of the entire host process.

Minimal repro shape

A ZIP archive containing a minimal valid [Content_Types].xml, _rels/.rels, xl/workbook.xml (+ relationships) sufficient for open_workbook_auto/Xlsx::new() to proceed as far as read_shared_strings, plus xl/sharedStrings.xml:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<sst xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" count="0" uniqueCount="4000000000"></sst>

No <si> elements are required for the reserve call to fire — it happens while parsing the <sst> start tag itself, before the second parsing loop (which reads <si> children) even begins.

Suggested fix

Apply the same guard pattern already used for the dimension-driven reservation, symmetrically, to uniqueCount: either skip the reserve call above some sane maximum (mirroring if len < 100_000), or switch to try_reserve(n) (which returns a Result/TryReserveError instead of aborting) combined with a sane maximum, so a legitimate large-but-plausible uniqueCount still gets a useful capacity hint while an implausible one fails gracefully rather than aborting the host process.

Downstream mitigation note

We've since implemented and tested a preflight validation layer in our own application (a live ZIP-entry probe measuring each entry's actual decompressed byte count via a capped reader — never trusting the declared uncompressed_size — plus a byte-level scan of sharedStrings.xml's root tag validating uniqueCount against a hard ceiling and the entry's own measured length) that closes this on our side regardless of upstream's response. We're filing this so the underlying library can close the gap at the source too — this is not a request to invent new defensive machinery, only to apply a pattern this codebase's own authors have already used elsewhere in the same file, symmetrically — and not a substitute for our own downstream hardening, nor something we're waiting on before shipping.

[jmcnamara]

Thanks for the report. That is indeed a bug.

Maintainers note: this is one of many similar issues we have had with with_capacity() and reserve(). Rust developers are actively encouraged to use these functions for performance reasons but for any condition where the capacity is interpreted from the input there is potential for an inadvertent or exploitable OOM issue. Rather than fixing just this one instance we should do a full audit of any other sites where it could happen.

[devsamhan]

Thanks for confirming.

I agree that a class-wide audit of reserve()/with_capacity() sites is the right approach, especially where the requested capacity is derived from workbook-controlled metadata.

For our downstream app, we have already added a local preflight layer before calling calamine for XLSX/XLSM and temporarily disabled legacy .xls import because our local review also found related allocation-risk patterns in the reachable CFB/XLS path. We are not relying on an upstream fix for our own safety, but I’m happy to help validate a calamine-side fix or test a patch/release candidate if useful.

[jmcnamara]

If you have found issues in the xls support then it would be good to raise those too, either here if they are related or in a separate bug report if that make more sense.

[devsamhan]

Thanks. Here are the related findings from our review of the legacy .xls path.

These were found by inspecting the published calamine 0.36.0 source. The line numbers below refer to that version.

1. CFB DIFAT count drives an unbounded allocation

In src/cfb.rs, Header::from_reader reads the declared DIFAT-sector count directly from the CFB header and uses it as the capacity of a vector:

let difat_len = read_usize(&buf[62..76]);
Vec::with_capacity(difat_len)

The value originates from an attacker-controlled u32 header field and is not checked against the file size, the number of sectors actually present, or a reasonable maximum before allocation.
At the maximum representable value, this can request capacity for roughly 4.29 billion u32 entries, which is on the order of 16 GiB in a single allocation request.
This happens during CFB header parsing, before the Workbook stream or any BIFF record is located.

2. CFB FAT count drives a second unbounded allocation
   In Cfb::new, the declared FAT-sector count from the same CFB header is used directly here:
   let mut fats = Vec::with_capacity(h.fat_len);
   h.fat_len is also parsed from a raw attacker-controlled u32 field, without first validating it against the real file length or maximum possible sector count.
   This is a separate allocation from the DIFAT allocation and has a similar worst-case order of magnitude.

3. The XLS Dimensions record drives an unguarded cell reservation
   In src/xls.rs, handling of the BIFF Dimensions record (0x0200) derives a row and column range from workbook-controlled record values and then performs:
   cells.reserve(rows.saturating_mul(cols));
   The declared dimensions are not required to correspond to the amount of real cell data that follows, and there is no practical upper bound before the reservation.
   This appears to be the .xls equivalent of the sharedStrings.xml uniqueCount issue: a small declared metadata value in the file controls a potentially very large allocation request.
   It is also notably less guarded than the analogous XLSX dimension-driven reservations, where calamine already avoids reserving when the calculated length is too large.

4. The root CFB directory chain is read with no length bound
   Cfb::new reads the root directory stream using:
   sectors.get_chain(h.dir_start, &fats, reader, usize::MAX)
   Passing usize::MAX means there is effectively no application-level maximum on the number of sectors accumulated for this chain.
   Because the next-sector values come from the file-controlled FAT, a malformed, cyclic, or deliberately very long chain may cause excessive traversal and memory accumulation.
   A deterministic upper bound could instead be derived from the number of sectors physically possible in the input file, with explicit cycle detection for already-visited sector IDs.

5. Two malformed-input panic paths
   The same review found two operations that appear able to panic on malformed CFB metadata rather than returning a parsing error:
   difat.pop().unwrap()
   and:
   fats[sector_id as usize]
   The first assumes that a DIFAT entry is present. The second performs unchecked indexing using a sector ID influenced by file metadata.
   These should preferably use checked handling and return a structured CFB/XLS error when the entry or sector index is invalid.

I have not yet constructed minimal standalone files proving that these two panic paths can be triggered independently of the allocation findings above, so I am reporting that limitation explicitly. They are source-level crash-path findings at this stage, not claimed here as completed reproducer cases.
Reachability
These paths are part of the normal legacy .xls opening flow:
.xls dispatches to Xls::new_with_options;
that invokes the CFB parser;
the CFB header allocations occur immediately;
parse_workbook() then parses the workbook and worksheets.
They are therefore reachable through an ordinary calamine .xls open, rather than being unused helper code.
Possible fixes
Potential hardening measures could include:
validate DIFAT and FAT counts against the actual file size and maximum possible sector count before reserving;
use try_reserve and return a parsing error instead of allowing allocation failure to terminate the process;
skip or cap the BIFF Dimensions-based reservation, following the guard pattern already used for large XLSX dimensions;
bound sector-chain traversal using the number of sectors physically present in the file;
track visited sector IDs to reject cycles;
replace unwrap and unchecked FAT indexing with checked errors.
In our downstream application we have temporarily disabled legacy .xls import, so we are not relying on an upstream fix for immediate protection.