shifathosn
/
gridcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

xlsx: bump quick-xml to 0.41 for the namespace-declaration DoS (RUSTSEC-2026-0195) 

The ODS and XLSB readers parse untrusted package metadata and content through
`quick_xml::NsReader`. In 0.38.4 that is RUSTSEC-2026-0195 (CVSS 7.5): the namespace
resolver pushes a binding for every `xmlns` / `xmlns:*` declaration on a start tag, with
no cap on how many there are, and `NsReader::read_event` reaches that path - not only
`read_resolved_event`.

None of our caps bound it. MAX_PART (512 MiB), MAX_TOTAL (1 GiB), MAX_ENTRIES,
MAX_CELLS, MAX_TEXT and MAX_DEPTH bound part size, entry count, cells, text and nesting,
never declarations per tag.

Measured with `gridcraft-cli info` on crafted .ods files, before this change:

     754 B, 0 declarations  ->    8.6 MB, exit 0
     24 KB, 1,000,000       ->   52.6 MB, exit 0
    187 KB, 8,000,000       ->  367.8 MB, exit 0
    770 KB, 33,000,000      ->    1.46 GB, exit 0

About 45 bytes of resident memory per declaration, roughly 1,900x the file size, and no
error at all. At MAX_PART a ~1 MB file reaches ~1.9 GB, and every part is held in memory
while the package is read. Both readers share the path, so a downloaded .ods or .xlsb
was enough. Nothing panicked, so `Session::execute` never saw it.

0.41.0 caps declarations per start tag at 256 (DEFAULT_MAX_DECLARATIONS_PER_ELEMENT,
raisable through NamespaceResolver::set_max_declarations_per_element) and returns
`NamespaceError::TooManyDeclarations` beyond it. The same fixtures after:

     754 B ->    8.6 MB, ok         24 KB ->   19 MB, error
    187 KB ->  101.8 MB, error     770 KB ->    397 MB, error

with "start tag declares more than 256 namespace bindings". The residual peak is the
part itself: MAX_PART still lets one part decompress to 256 MiB, the existing intended
bound.

API changes for 0.41 (0.x bumps may break; all four are in the xlsx crate):
- `resolve_element` and `resolve_attribute` moved off `NsReader` on to the resolver:
  `reader.resolver_mut().resolve_element(..)`. `attr` and `tag` in ods.rs take
  `&mut NsReader` for that.
- `Attribute::unescape_value` is deprecated in favour of `normalized_value`, which also
  applies the attribute-value normalization the XML specification requires.
- `BytesText::xml_content` now takes the document's XML version.

This also clears RUSTSEC-2026-0194, the quadratic duplicate-attribute check. xml.rs
already passed `with_checks(false)`, but ods.rs and xlsb.rs used the checked iterator, so
they were on the quadratic path as well.

Regression test: `ods_tests::ods_namespace_declaration_bomb_is_rejected` asserts that a
tag with thousands of declarations is an error and that a document with a handful still
reads. It fails on 0.38.4 (the tag is accepted) and passes on 0.41.0.

Verified: `cargo xtask ci`; the full xlsx suite (80 tests) plus the malformed-input
tests; and the four measurements above, before and after, with no panics either way.
This commit is contained in:
pkibiego 2026-10-11 03:39:32 +03:00
parent 9f896c1249
commit 2f078962a0
6 changed files with 38 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
Cargo.lock generated
View File

@ -1584,7 +1584,7 @@ dependencies = [
"gridcraft-core",
"gridcraft-formula",
"gridcraft-model",
"quick-xml 0.38.4",
"quick-xml",
"thiserror 2.0.21",
"zip",
]
@ -2739,15 +2739,6 @@ version = "2.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3"
[[package]]
name = "quick-xml"
version = "0.38.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b66c2058c55a409d601666cffe35f04333cf1013010882cec174a7467cd4e21c"
dependencies = [
"memchr",
]
[[package]]
name = "quick-xml"
version = "0.41.0"
@ -3725,7 +3716,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "338e30461b3a2b67d70eb30a6d89f8e0c93a833e07d2ae89085cd070c4a00ac0"
dependencies = [
"proc-macro2",
"quick-xml 0.41.0",
"quick-xml",
"quote",
]

2
Cargo.toml
View File

@ -32,7 +32,7 @@ serde_json = { version = "1", features = ["float_roundtrip"] }
log = "0.4"
rayon = "1"
regex = { version = "1", default-features = false, features = ["std", "unicode", "perf"] }
quick-xml = "0.38"
quick-xml = "0.41"
zip = { version = "3", default-features = false, features = ["deflate"] }
image = { version = "0.25", default-features = false, features = ["png", "jpeg"] }

23
crates/xlsx/src/ods.rs
View File

@ -10,6 +10,7 @@ use gridcraft_core::date::{days_in_month, serial_from_ymd};
use gridcraft_core::{CellRef, DateSystem, MAX_COLS, MAX_ROWS, RangeRef, Value};
use gridcraft_model::{Cell, NumFmt, Sheet, StyleId, Workbook};
use quick_xml::NsReader;
use quick_xml::XmlVersion;
use quick_xml::events::{BytesStart, Event};
use quick_xml::name::ResolveResult;
@ -67,13 +68,13 @@ fn is_ns(ns: ResolveResult<'_>, expected: &[u8]) -> bool {
matches!(ns, ResolveResult::Bound(n) if n.as_ref() == expected)
}
fn attr(reader: &NsReader<&[u8]>, e: &BytesStart<'_>, ns: &[u8], name: &[u8]) -> Result<Option<String>, IoError> {
fn attr(reader: &mut NsReader<&[u8]>, e: &BytesStart<'_>, ns: &[u8], name: &[u8]) -> Result<Option<String>, IoError> {
let mut found = None;
for a in e.attributes() {
let a = a.map_err(|e| IoError::Xml(e.to_string()))?;
let (resolved, local) = reader.resolve_attribute(a.key);
let (resolved, local) = reader.resolver_mut().resolve_attribute(a.key);
if is_ns(resolved, ns) && local.as_ref() == name {
let value = a.unescape_value().map_err(|e| IoError::Xml(e.to_string()))?;
let value = a.normalized_value(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?;
if value.len() > MAX_CELL_TEXT {
return Err(IoError::TooLarge("ODS attribute exceeds 1 MiB".into()));
}
@ -106,7 +107,7 @@ fn check_manifest(bytes: &[u8]) -> Result<(), IoError> {
let event = reader.read_event().map_err(xml_error)?;
match &event {
Event::Start(e) | Event::Empty(e) => {
let (ns, name) = reader.resolve_element(e.name());
let (ns, name) = reader.resolver_mut().resolve_element(e.name());
if is_ns(ns.clone(), MANIFEST) && name.as_ref() == b"encryption-data" {
return Err(IoError::Format("encrypted ODS files are not supported".into()));
}
@ -153,8 +154,8 @@ enum Tag {
Other,
}
fn tag(reader: &NsReader<&[u8]>, e: &BytesStart<'_>) -> Tag {
let (ns, name) = reader.resolve_element(e.name());
fn tag(reader: &mut NsReader<&[u8]>, e: &BytesStart<'_>) -> Tag {
let (ns, name) = reader.resolver_mut().resolve_element(e.name());
let local = name.as_ref();
if is_ns(ns.clone(), OFFICE) {
return match local {
@ -253,23 +254,23 @@ impl Import {
return Err(xml_error("XML nested too deeply"));
}
if self.stack.is_empty() {
if root_seen || tag(&reader, &e) != Tag::Document {
if root_seen || tag(&mut reader, &e) != Tag::Document {
return Err(xml_error("expected one office:document-content root"));
}
root_seen = true;
}
self.start(&reader, &e)?;
self.start(&mut reader, &e)?;
}
Event::Empty(e) => {
if self.stack.is_empty() {
return Err(xml_error("expected office:document-content with a spreadsheet"));
}
self.start(&reader, &e)?;
self.start(&mut reader, &e)?;
self.end()?;
}
Event::End(_) => self.end()?,
Event::Text(t) => {
let text = t.xml_content().map_err(xml_error)?;
let text = t.xml_content(XmlVersion::Explicit1_0).map_err(xml_error)?;
if self.stack.is_empty() && !text.trim().is_empty() {
return Err(xml_error("text outside the document"));
}
@ -301,7 +302,7 @@ impl Import {
Ok(())
}
fn start(&mut self, reader: &NsReader<&[u8]>, e: &BytesStart<'_>) -> Result<(), IoError> {
fn start(&mut self, reader: &mut NsReader<&[u8]>, e: &BytesStart<'_>) -> Result<(), IoError> {
let t = tag(reader, e);
let depth = self.stack.len();
match t {

16
crates/xlsx/src/tests/ods_tests.rs
View File

@ -23,6 +23,22 @@ fn text(s: &str) -> Value {
Value::Text(s.into())
}
/// A start tag with thousands of `xmlns:` declarations is refused instead of allocated
/// (RUSTSEC-2026-0195: the resolver kept a binding per declaration, with no cap).
#[test]
fn ods_namespace_declaration_bomb_is_rejected() {
let bomb = |n: usize| -> String { (0..n).map(|i| format!(" xmlns:p{i}=\"urn:p{i}\"")).collect() };
let doc = |decls: &str| {
format!(
r#"<?xml version="1.0"?><office:document-content xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:table="urn:oasis:names:tc:opendocument:xmlns:table:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0"{decls}><office:body><office:spreadsheet><table:table table:name="Data"><table:table-row><table:table-cell office:value-type="float" office:value="1"><text:p>1</text:p></table:table-cell></table:table-row></table:table></office:spreadsheet></office:body></office:document-content>"#
)
};
let build = |decls: &str| make_zip(&[("mimetype", MIME), ("META-INF/manifest.xml", MANIFEST), ("content.xml", doc(decls).as_bytes())]);
let ok = read_ods(&build(&bomb(8)));
assert!(ok.is_ok(), "a handful of namespaces still reads: {:?}", ok.err());
assert!(read_ods(&build(&bomb(10_000))).is_err(), "a tag with thousands of declarations is refused");
}
#[test]
fn ods_typed_values_and_formula_caches_are_constants_and_survive_xlsx() {
let bytes = row(r#"

7
crates/xlsx/src/xlsb.rs
View File

@ -7,6 +7,7 @@ use std::sync::Arc;
use gridcraft_core::{CellError, CellRef, DateSystem, MAX_COLS, MAX_ROWS, RangeRef, Value};
use gridcraft_model::{Cell, Sheet, Visibility, Workbook};
use quick_xml::NsReader;
use quick_xml::XmlVersion;
use quick_xml::events::Event;
use quick_xml::name::ResolveResult;
@ -160,7 +161,7 @@ fn metadata(bytes: &[u8], ns: &[u8], root_name: &[u8]) -> Result<Vec<Entry>, IoE
let event = reader.read_event().map_err(|e| IoError::Xml(e.to_string()))?;
match &event {
Event::Start(e) | Event::Empty(e) => {
let (namespace, local) = reader.resolve_element(e.name());
let (namespace, local) = reader.resolver_mut().resolve_element(e.name());
let valid_ns = matches!(namespace, ResolveResult::Bound(n) if n.as_ref() == ns);
if depth == 0 {
if seen || !valid_ns || local.as_ref() != root_name {
@ -174,7 +175,7 @@ fn metadata(bytes: &[u8], ns: &[u8], root_name: &[u8]) -> Result<Vec<Entry>, IoE
if !a.key.as_ref().contains(&b':') {
attrs.push((
String::from_utf8_lossy(a.key.as_ref()).into_owned(),
a.unescape_value().map_err(|e| IoError::Xml(e.to_string()))?.into_owned(),
a.normalized_value(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?.into_owned(),
));
}
}
@ -188,7 +189,7 @@ fn metadata(bytes: &[u8], ns: &[u8], root_name: &[u8]) -> Result<Vec<Entry>, IoE
}
}
Event::End(_) => depth = depth.checked_sub(1).ok_or_else(|| invalid("unexpected metadata end"))?,
Event::Text(t) if depth == 0 && !t.xml_content().map_err(|e| IoError::Xml(e.to_string()))?.trim().is_empty() => {
Event::Text(t) if depth == 0 && !t.xml_content(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?.trim().is_empty() => {
return Err(invalid("text outside package metadata"));
}
Event::DocType(_) => return Err(invalid("DOCTYPE in package metadata is unsupported")),

5
crates/xlsx/src/xml.rs
View File

@ -2,6 +2,7 @@
//! shared strings) so large parts never need a full tree, plus escaping helpers for writing.
use quick_xml::Reader;
use quick_xml::XmlVersion;
use quick_xml::events::{BytesStart, Event};
use crate::IoError;
@ -124,7 +125,7 @@ fn start_el(e: &BytesStart<'_>) -> El {
if key == b"xmlns" || key.starts_with(b"xmlns:") {
continue;
}
let v = match a.unescape_value() {
let v = match a.normalized_value(XmlVersion::Explicit1_0) {
Ok(v) => v.into_owned(),
Err(_) => String::from_utf8_lossy(&a.value).into_owned(),
};
@ -203,7 +204,7 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res
}
Event::Text(t) => {
if let Some(top) = stack.last_mut() {
let s = t.xml_content().map_err(|e| IoError::Xml(e.to_string()))?;
let s = t.xml_content(XmlVersion::Explicit1_0).map_err(|e| IoError::Xml(e.to_string()))?;
top.text.push_str(&s);
}
}