A <dimension> whose end cell precedes its start underflows two unchecked u32 subtractions. ECMA-376 does not require ref to be ordered, so this is reachable from a file a writer can legitimately emit, and from any untrusted input.
Two sites, same root, both still present on master:
src/xlsx/mod.rs:2789-2790 — let rows = parts[1].0 - parts[0].0; and the column line under it
src/lib.rs:181-183 — Dimensions::len() → (self.end.0 - self.start.0 + 1) as u64 * ...
Repro
A minimal OPC package (one sheet, no shared strings, no styles) whose xl/worksheets/sheet1.xml is:
<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main">
<dimension ref="C5:A1"/>
<sheetData>
<row r="1"><c r="A1"><v>1</v></c></row>
<row r="5"><c r="C5"><v>9</v></c></row>
</sheetData>
</worksheet>
let mut wb: Xlsx<_> = open_workbook(&path)?;
let name = wb.sheet_names()[0].clone();
let dims = wb.worksheet_cells_reader(&name)?.dimensions();
println!("declared start={:?} end={:?}", dims.start, dims.end);
println!("Dimensions::len() = {}", dims.len());
Debug build (calamine 0.36.0):
thread 'main' panicked at src/xlsx/mod.rs:2789:24:
attempt to subtract with overflow
Release build:
declared start=(4, 2) end=(0, 0)
Dimensions::len() = 18446744056529682435
Why it is worth fixing rather than documenting
worksheet_range masks this in release, because from_sparse rebuilds the extent from the cells it actually saw. The damage surfaces through dimensions(), which is the natural pre-allocation hint for the incremental readers, and through Dimensions::len(), which is public API with no documented precondition. Reading the declared extent and calling len() on it is the obvious thing to do, and it silently yields 1.8e19.
This is the same shape as #150 and #174, which were fixed in the xls path. The xlsx path and the public Dimensions type still have it.
Suggested fix
Normalise the corners in get_dimension, or reject the ref outright, or make the public computation total:
pub fn len(&self) -> u64 {
if self.end.0 < self.start.0 || self.end.1 < self.start.1 {
return 0;
}
u64::from(self.end.0 - self.start.0 + 1) * u64::from(self.end.1 - self.start.1 + 1)
}
Found while building the gRPC service in #688, which streams cells through worksheet_cells_reader and reports dimensions() to clients. Glad to send a PR if you have a preference for which of the three directions you want.
A
<dimension>whose end cell precedes its start underflows two uncheckedu32subtractions. ECMA-376 does not requirerefto be ordered, so this is reachable from a file a writer can legitimately emit, and from any untrusted input.Two sites, same root, both still present on master:
src/xlsx/mod.rs:2789-2790—let rows = parts[1].0 - parts[0].0;and the column line under itsrc/lib.rs:181-183—Dimensions::len()→(self.end.0 - self.start.0 + 1) as u64 * ...Repro
A minimal OPC package (one sheet, no shared strings, no styles) whose
xl/worksheets/sheet1.xmlis:Debug build (calamine 0.36.0):
Release build:
Why it is worth fixing rather than documenting
worksheet_rangemasks this in release, becausefrom_sparserebuilds the extent from the cells it actually saw. The damage surfaces throughdimensions(), which is the natural pre-allocation hint for the incremental readers, and throughDimensions::len(), which is public API with no documented precondition. Reading the declared extent and callinglen()on it is the obvious thing to do, and it silently yields 1.8e19.This is the same shape as #150 and #174, which were fixed in the xls path. The xlsx path and the public
Dimensionstype still have it.Suggested fix
Normalise the corners in
get_dimension, or reject the ref outright, or make the public computation total:Found while building the gRPC service in #688, which streams cells through
worksheet_cells_readerand reportsdimensions()to clients. Glad to send a PR if you have a preference for which of the three directions you want.