From 93a30a9cb82e01f5e2a2d8f08c298d6c0a8ee570 Mon Sep 17 00:00:00 2001 From: scadastrangelove Date: Sun, 19 Jul 2026 12:02:56 +0300 Subject: [PATCH 1/3] Bound NamespaceResolver nesting depth (fix #977) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `NamespaceResolver::push` incremented a `u16` `nesting_level` with an unguarded `+= 1`; a document nested past `u16::MAX` overflowed it — a panic under `overflow-checks`, or a silent wrap that corrupts namespace-scope truncation in release (bindings from closed scopes leak / in-scope bindings vanish). `pop` already saturates; the increment did not. Return a new `NamespaceError::TooDeeplyNested` at the `u16` boundary instead, matching the existing per-element `TooManyDeclarations` guard. A `saturating_add` would stop the panic but not the scope corruption (at saturation deep levels collapse and `set_level` still over-truncates). Co-Authored-By: Claude Opus 4.8 --- Changelog.md | 8 ++++++++ src/name.rs | 30 +++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 060a381f..b90f2c0c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -18,6 +18,13 @@ ### Bug Fixes +- [#977]: `NamespaceResolver::push` (and hence every `NsReader` `Start`/`Empty` + event) now returns the new `NamespaceError::TooDeeplyNested` when a document + nests elements deeper than `u16::MAX`, instead of overflowing the internal + `u16` depth counter. Previously the unguarded `nesting_level += 1` panicked + under `overflow-checks` builds and silently wrapped in release, corrupting + namespace-scope bookkeeping on deeply nested untrusted input. + ### Misc Changes @@ -47,6 +54,7 @@ [#969]: https://github.com/tafia/quick-xml/issues/969 [#970]: https://github.com/tafia/quick-xml/issues/970 +[#977]: https://github.com/tafia/quick-xml/issues/977 ## 0.40.1 -- 2026-05-15 diff --git a/src/name.rs b/src/name.rs index ab92a4c6..0160c203 100644 --- a/src/name.rs +++ b/src/name.rs @@ -47,6 +47,10 @@ pub enum NamespaceError { /// This bounds the heap allocated by [`NamespaceResolver::push`] (and hence /// by [`NsReader`](crate::reader::NsReader)) on untrusted input. TooManyDeclarations(usize), + /// The document nested elements more deeply than the namespace resolver's + /// depth counter (a `u16`) can track. This bounds stack / scope-bookkeeping + /// work on untrusted input. Contains the depth limit that was exceeded. + TooDeeplyNested(usize), } impl fmt::Display for NamespaceError { @@ -85,6 +89,9 @@ impl fmt::Display for NamespaceError { limit, ) } + Self::TooDeeplyNested(limit) => { + write!(f, "document nests elements deeper than the supported limit of {}", limit) + } } } } @@ -706,7 +713,9 @@ impl NamespaceResolver { /// /// [namespace bindings]: https://www.w3.org/TR/xml-names11/#dt-NSDecl pub fn push(&mut self, start: &BytesStart) -> Result<(), NamespaceError> { - self.nesting_level += 1; + self.nesting_level = self.nesting_level + .checked_add(1) + .ok_or(NamespaceError::TooDeeplyNested(u16::MAX as usize))?; let mut count = 0usize; // adds new namespaces for attributes starting with 'xmlns:' and for the 'xmlns' // (default namespace) attribute. @@ -1304,6 +1313,25 @@ mod namespaces { ); } + /// Regression test for : + /// `push()` previously incremented a `u16` depth counter with an unguarded + /// `+= 1`, so a document nested past `u16::MAX` panicked under + /// `overflow-checks` or silently wrapped the counter and corrupted namespace + /// scoping in release. It now returns `TooDeeplyNested` at the boundary. + #[test] + fn push_rejects_pathological_nesting_depth() { + let mut resolver = NamespaceResolver::default(); + let tag = BytesStart::from_content("a", 1); + // `u16::MAX` successful pushes, then the next is rejected cleanly. + for _ in 0..u16::MAX { + assert_eq!(resolver.push(&tag), Ok(())); + } + assert_eq!( + resolver.push(&tag), + Err(NamespaceError::TooDeeplyNested(u16::MAX as usize)), + ); + } + /// Unprefixed attribute names (resolved with `false` flag) never have a namespace /// according to : /// From 284e477281b7831bf3561913c3216cca42a4a464 Mon Sep 17 00:00:00 2001 From: scadastrangelove Date: Sun, 19 Jul 2026 14:23:20 +0300 Subject: [PATCH 2/3] Move #977 changelog reference into the Unreleased section Per review: the reference link belonged with its Unreleased entry, not under the already-released 0.41.0 section. --- Changelog.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index b90f2c0c..0b56fe07 100644 --- a/Changelog.md +++ b/Changelog.md @@ -27,6 +27,8 @@ ### Misc Changes +[#977]: https://github.com/tafia/quick-xml/issues/977 + ## 0.41.0 -- 2026-06-29 @@ -54,7 +56,6 @@ [#969]: https://github.com/tafia/quick-xml/issues/969 [#970]: https://github.com/tafia/quick-xml/issues/970 -[#977]: https://github.com/tafia/quick-xml/issues/977 ## 0.40.1 -- 2026-05-15 From 2fdf1aadfb8017c768042d67c625f727954bbe99 Mon Sep 17 00:00:00 2001 From: scadastrangelove Date: Mon, 20 Jul 2026 09:37:28 +0300 Subject: [PATCH 3/3] cargo fmt --- src/name.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/name.rs b/src/name.rs index 0160c203..12b8e0aa 100644 --- a/src/name.rs +++ b/src/name.rs @@ -90,7 +90,11 @@ impl fmt::Display for NamespaceError { ) } Self::TooDeeplyNested(limit) => { - write!(f, "document nests elements deeper than the supported limit of {}", limit) + write!( + f, + "document nests elements deeper than the supported limit of {}", + limit + ) } } } @@ -713,7 +717,8 @@ impl NamespaceResolver { /// /// [namespace bindings]: https://www.w3.org/TR/xml-names11/#dt-NSDecl pub fn push(&mut self, start: &BytesStart) -> Result<(), NamespaceError> { - self.nesting_level = self.nesting_level + self.nesting_level = self + .nesting_level .checked_add(1) .ok_or(NamespaceError::TooDeeplyNested(u16::MAX as usize))?; let mut count = 0usize;