From 92d1a10d452ae0b228210c0644fa4ce2c90cb4da Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Fri, 3 Jul 2026 01:37:54 +0200 Subject: [PATCH] security(quick-xml): bump version to 0.41.0 - and protect amount of properties - azure_core 0.21.0 is using quick-xml 0.31.0 which is Dos-able azure_core is no more maintained, would migrte to official azure lib later --- .cargo/audit.toml | 13 +++++ Cargo.lock | 6 +-- Cargo.toml | 2 +- src/application/adapters/caldav_adapter.rs | 8 ++- src/application/adapters/webdav_adapter.rs | 50 +++++++++++++++++-- .../services/search_index/text_extractor.rs | 5 +- .../services/wopi_discovery_service.rs | 5 +- 7 files changed, 78 insertions(+), 11 deletions(-) diff --git a/.cargo/audit.toml b/.cargo/audit.toml index 51ac2ffc..e2559997 100644 --- a/.cargo/audit.toml +++ b/.cargo/audit.toml @@ -26,4 +26,17 @@ ignore = [ # instant unmaintained — transitive via azure_core 0.21.0 (latest available). # No direct security impact; no upgrade path exists. "RUSTSEC-2024-0384", + + # quick-xml 0.31.0 — transitive via azure_core 0.21.0 (unofficial SDK, + # now archived). Our direct dep is already on 0.41.0; the 0.31 copy is + # only reachable through the azure_storage_blobs chain, which parses + # XML responses received from Azure Storage over TLS. Neither CVE is + # exploitable without attacker-controlled XML, so the vector requires + # MitM of the TLS channel to Azure (or a compromised storage + # endpoint). Real fix is migrating to the official azure_core 1.0 / + # azure_storage_blob 1.0 SDK — tracked separately. + # RUSTSEC-2026-0195: unbounded ns-declaration allocation → mem-DoS + # RUSTSEC-2026-0194: quadratic dup-attribute check → CPU-DoS + "RUSTSEC-2026-0195", + "RUSTSEC-2026-0194", ] diff --git a/Cargo.lock b/Cargo.lock index a5f9438c..81fba5fa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4267,7 +4267,7 @@ dependencies = [ "ort", "pdf-extract", "percent-encoding", - "quick-xml 0.39.4", + "quick-xml 0.41.0", "rand_core 0.6.4", "rayon", "reqwest", @@ -4743,9 +4743,9 @@ dependencies = [ [[package]] name = "quick-xml" -version = "0.39.4" +version = "0.41.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cdcc8dd4e2f670d309a5f0e83fe36dfdc05af317008fea29144da1a2ac858e5e" +checksum = "e660451e55124f798a69a5af3f49ccfbefbd41910eefd25caf2393e1f3473ec1" dependencies = [ "memchr", ] diff --git a/Cargo.toml b/Cargo.toml index ac29094c..d01b82f1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,7 @@ sqlx = { version = "0.8.6", features = ["postgres", "runtime-tokio", "tls-rustls jsonwebtoken = { version = "10.4.0", features = ["rust_crypto"] } argon2 = "0.5.3" rand_core = { version = "0.6", features = ["std", "getrandom"] } -quick-xml = "0.39.4" +quick-xml = "0.41.0" dotenvy = "0.15.7" moka = { version = "0.12.15", features = ["future", "sync"] } http-range-header = "0.4" diff --git a/src/application/adapters/caldav_adapter.rs b/src/application/adapters/caldav_adapter.rs index e7e67cff..b2541e2f 100644 --- a/src/application/adapters/caldav_adapter.rs +++ b/src/application/adapters/caldav_adapter.rs @@ -96,7 +96,9 @@ impl CalDavAdapter { for attr in e.attributes().flatten() { let attr_name = std::str::from_utf8(attr.key.as_ref()).unwrap_or(""); - let attr_value = attr.unescape_value().unwrap_or_default(); + let attr_value = attr + .normalized_value(quick_xml::XmlVersion::Implicit1_0) + .unwrap_or_default(); if attr_name == "start" { // Parse ISO date format with Z for UTC @@ -161,7 +163,9 @@ impl CalDavAdapter { // Parse time-range attributes for attr in e.attributes().flatten() { let attr_name = std::str::from_utf8(attr.key.as_ref()).unwrap_or(""); - let attr_value = attr.unescape_value().unwrap_or_default(); + let attr_value = attr + .normalized_value(quick_xml::XmlVersion::Implicit1_0) + .unwrap_or_default(); if attr_name == "start" { // Parse ISO date format with Z for UTC diff --git a/src/application/adapters/webdav_adapter.rs b/src/application/adapters/webdav_adapter.rs index 9cd063b1..6731e185 100644 --- a/src/application/adapters/webdav_adapter.rs +++ b/src/application/adapters/webdav_adapter.rs @@ -183,10 +183,42 @@ impl NextcloudPropContext { } } +/// Defense-in-depth cap on attributes per XML element in WebDAV request +/// bodies. Legitimate PROPFIND / PROPPATCH elements carry a handful of +/// `xmlns:*` declarations and, occasionally, per-property namespace +/// bindings — a dozen is already a lot. 100 is generous headroom and +/// three orders of magnitude below what an attacker would need to +/// exploit a quadratic parser bug (see quick-xml #969, fixed in 0.41; +/// this cap fences the same threat model for any future analogous bug +/// in whatever parser we swap to). +/// +/// A rejected element yields 400 Bad Request via the ParseError path. +pub const MAX_ATTRIBUTES_PER_ELEMENT: usize = 100; + /// WebDAV adapter for converting between XML and domain objects pub struct WebDavAdapter; impl WebDavAdapter { + /// Refuse elements carrying an unreasonable attribute count. + /// See [`MAX_ATTRIBUTES_PER_ELEMENT`] for the reasoning. + /// + /// `Attributes::count()` is O(N) in the number of attributes (each + /// attribute is parsed once), so this check itself is safe even + /// against very large elements. The parser may still have paid a + /// quadratic cost by the time we get here on a vulnerable version + /// of the underlying library — the bump to quick-xml 0.41 closes + /// that specific bug; this cap is defense-in-depth against future + /// analogous bugs and against adversarially large XML that would + /// otherwise reach our downstream code. + fn check_attribute_cap(e: &BytesStart) -> Result<()> { + if e.attributes().count() > MAX_ATTRIBUTES_PER_ELEMENT { + return Err(WebDavError::ParseError(format!( + "Element carries more than {MAX_ATTRIBUTES_PER_ELEMENT} attributes" + ))); + } + Ok(()) + } + /// Collect namespace prefix → URI mappings from element attributes. /// E.g. `xmlns:D="DAV:"` maps prefix `"D"` to `"DAV:"`. pub fn collect_ns_decls( @@ -196,11 +228,17 @@ impl WebDavAdapter { for attr in e.attributes().flatten() { let key = std::str::from_utf8(attr.key.as_ref()).unwrap_or(""); if let Some(prefix) = key.strip_prefix("xmlns:") { - let uri = attr.unescape_value().unwrap_or_default().to_string(); + let uri = attr + .normalized_value(quick_xml::XmlVersion::Implicit1_0) + .unwrap_or_default() + .to_string(); ns_map.insert(prefix.to_string(), uri); } else if key == "xmlns" { // Default namespace declaration: xmlns="uri" - let uri = attr.unescape_value().unwrap_or_default().to_string(); + let uri = attr + .normalized_value(quick_xml::XmlVersion::Implicit1_0) + .unwrap_or_default() + .to_string(); ns_map.insert(String::new(), uri); } } @@ -212,7 +250,9 @@ impl WebDavAdapter { for attr in e.attributes().flatten() { let key = std::str::from_utf8(attr.key.as_ref()).unwrap_or(""); if key.starts_with("xmlns:") { - let uri = attr.unescape_value().unwrap_or_default(); + let uri = attr + .normalized_value(quick_xml::XmlVersion::Implicit1_0) + .unwrap_or_default(); if uri.is_empty() { return Err(WebDavError::ParseError( "Invalid namespace declaration: prefix bound to empty URI".to_string(), @@ -265,6 +305,7 @@ impl WebDavAdapter { loop { match xml_reader.read_event_into(&mut buffer) { Ok(Event::Start(ref e)) => { + Self::check_attribute_cap(e)?; Self::collect_ns_decls(e, &mut ns_map); Self::check_ns_decls_valid(e)?; let name = e.name(); @@ -303,6 +344,7 @@ impl WebDavAdapter { } } Ok(Event::Empty(ref e)) => { + Self::check_attribute_cap(e)?; Self::collect_ns_decls(e, &mut ns_map); Self::check_ns_decls_valid(e)?; let name = e.name(); @@ -936,6 +978,7 @@ impl WebDavAdapter { loop { match xml_reader.read_event_into(&mut buffer) { Ok(Event::Start(ref e)) => { + Self::check_attribute_cap(e)?; Self::collect_ns_decls(e, &mut ns_map); let name = e.name(); let name_str = std::str::from_utf8(name.as_ref()).unwrap_or(""); @@ -1018,6 +1061,7 @@ impl WebDavAdapter { } } Ok(Event::Empty(ref e)) => { + Self::check_attribute_cap(e)?; Self::collect_ns_decls(e, &mut ns_map); let name = e.name(); let name_str = std::str::from_utf8(name.as_ref()).unwrap_or(""); diff --git a/src/infrastructure/services/search_index/text_extractor.rs b/src/infrastructure/services/search_index/text_extractor.rs index 470986aa..90b89ad8 100644 --- a/src/infrastructure/services/search_index/text_extractor.rs +++ b/src/infrastructure/services/search_index/text_extractor.rs @@ -243,7 +243,10 @@ fn collect_xml_text( } match xml.read_event_into(&mut buf) { Ok(Event::Text(t)) => { - if let Ok(decoded) = t.xml_content() { + // quick-xml 0.41+ makes XmlVersion explicit on xml_content() + // so callers pick 1.0 vs 1.1 entity-normalization rules. Text + // extraction is version-agnostic — 1.0 is the sane default. + if let Ok(decoded) = t.xml_content(quick_xml::XmlVersion::Implicit1_0) { out.push_str(&decoded); } } diff --git a/src/infrastructure/services/wopi_discovery_service.rs b/src/infrastructure/services/wopi_discovery_service.rs index 28eb04d9..27c127c7 100644 --- a/src/infrastructure/services/wopi_discovery_service.rs +++ b/src/infrastructure/services/wopi_discovery_service.rs @@ -203,7 +203,10 @@ impl WopiDiscoveryService { for attr in e.attributes().flatten() { let value = attr - .decode_and_unescape_value(reader.decoder()) + .decoded_and_normalized_value( + quick_xml::XmlVersion::Implicit1_0, + reader.decoder(), + ) .map(|value| value.into_owned()) .unwrap_or_else(|_| String::from_utf8_lossy(&attr.value).to_string());