Skip to content

Commit 7ca2526

Browse files
Qifan Zhangclaude
authored andcommitted
Cap namespace declarations per element in NamespaceResolver::push
`NsReader` calls `NamespaceResolver::push` for every `Start`/`Empty` event *before* the event is returned to the caller. `push` previously iterated all `xmlns` / `xmlns:*` attributes on the start tag and allocated one `NamespaceBinding` (plus prefix/value bytes in `buffer`) per declaration with no upper bound, so an `NsReader` consumer had no opportunity to bound its memory exposure on untrusted input — a start tag of M bytes drove ~3.3×M bytes of resolver heap that the caller never saw. With several concurrent readers this is a process-fatal OOM. Add a configurable per-element declaration limit (default 256, far above any real-world dialect) and a new `NamespaceError::TooManyDeclarations` variant. `push` now returns the error instead of allocating past the limit. Expose the limit via `NamespaceResolver::{max_declarations_per_element,set_max_declarations_per_element}` and add `NsReader::resolver_mut()` so callers can raise or disable it. Fixes #970. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 9aaea92 commit 7ca2526

3 files changed

Lines changed: 138 additions & 0 deletions

File tree

Changelog.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,23 @@
1616

1717
### New Features
1818

19+
- [#970]: Add `NsReader::resolver_mut()` and
20+
`NamespaceResolver::{max_declarations_per_element, set_max_declarations_per_element}`.
21+
1922
### Bug Fixes
2023

24+
- [#970]: `NamespaceResolver::push` (and hence every `NsReader` `Start`/`Empty`
25+
event) now rejects a start tag that declares more than
26+
`DEFAULT_MAX_DECLARATIONS_PER_ELEMENT` (256) `xmlns` / `xmlns:*` namespace
27+
bindings, returning the new `NamespaceError::TooManyDeclarations`. Previously
28+
`push` allocated one `NamespaceBinding` per declaration with no upper bound,
29+
before the event was returned to the caller, so an `NsReader` consumer could
30+
not bound its memory exposure on untrusted input. The limit is configurable
31+
via `NamespaceResolver::set_max_declarations_per_element` (use `usize::MAX`
32+
to disable).
33+
34+
[#970]: https://github.com/tafia/quick-xml/issues/970
35+
2136
### Misc Changes
2237

2338

src/name.rs

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ pub enum NamespaceError {
4040
///
4141
/// Contains the prefix that is tried to be bound.
4242
InvalidPrefixForXmlns(Vec<u8>),
43+
/// A single start tag declared more `xmlns` / `xmlns:*` namespace bindings
44+
/// than the configured [`NamespaceResolver::max_declarations_per_element`]
45+
/// limit. Contains the configured limit.
46+
///
47+
/// This bounds the heap allocated by [`NamespaceResolver::push`] (and hence
48+
/// by [`NsReader`](crate::reader::NsReader)) on untrusted input.
49+
TooManyDeclarations(usize),
4350
}
4451

4552
impl fmt::Display for NamespaceError {
@@ -70,6 +77,14 @@ impl fmt::Display for NamespaceError {
7077
write_byte_string(f, prefix)?;
7178
f.write_str("' cannot be bound to 'http://www.w3.org/2000/xmlns/'")
7279
}
80+
Self::TooManyDeclarations(limit) => {
81+
write!(
82+
f,
83+
"start tag declares more than {} namespace bindings; \
84+
raise the limit with NamespaceResolver::set_max_declarations_per_element",
85+
limit,
86+
)
87+
}
7388
}
7489
}
7590
}
@@ -497,14 +512,32 @@ pub struct NamespaceResolver {
497512
/// The number of open tags at the moment. We need to keep track of this to know which namespace
498513
/// declarations to remove when we encounter an `End` event.
499514
nesting_level: u16,
515+
/// Maximum number of `xmlns` / `xmlns:*` declarations [`push`](Self::push)
516+
/// will accept on a single start tag before returning
517+
/// [`NamespaceError::TooManyDeclarations`]. See
518+
/// [`set_max_declarations_per_element`](Self::set_max_declarations_per_element).
519+
max_declarations_per_element: usize,
500520
}
501521

522+
/// Default limit on the number of `xmlns` / `xmlns:*` declarations
523+
/// [`NamespaceResolver::push`] will accept on a single start tag.
524+
///
525+
/// Real-world XML dialects (XHTML, SVG, SOAP, RSS, RRDP, ...) declare a handful
526+
/// of namespaces per element; 256 is orders of magnitude above any legitimate
527+
/// document while bounding the heap allocated for one `<... xmlns:...>` tag to
528+
/// a few kilobytes regardless of input size.
529+
pub const DEFAULT_MAX_DECLARATIONS_PER_ELEMENT: usize = 256;
530+
502531
impl Debug for NamespaceResolver {
503532
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
504533
f.debug_struct("NamespaceResolver")
505534
.field("buffer", &Bytes(&self.buffer))
506535
.field("bindings", &self.bindings)
507536
.field("nesting_level", &self.nesting_level)
537+
.field(
538+
"max_declarations_per_element",
539+
&self.max_declarations_per_element,
540+
)
508541
.finish()
509542
}
510543
}
@@ -555,6 +588,7 @@ impl Default for NamespaceResolver {
555588
buffer,
556589
bindings,
557590
nesting_level: 0,
591+
max_declarations_per_element: DEFAULT_MAX_DECLARATIONS_PER_ELEMENT,
558592
}
559593
}
560594
}
@@ -673,11 +707,18 @@ impl NamespaceResolver {
673707
/// [namespace bindings]: https://www.w3.org/TR/xml-names11/#dt-NSDecl
674708
pub fn push(&mut self, start: &BytesStart) -> Result<(), NamespaceError> {
675709
self.nesting_level += 1;
710+
let mut count = 0usize;
676711
// adds new namespaces for attributes starting with 'xmlns:' and for the 'xmlns'
677712
// (default namespace) attribute.
678713
for a in start.attributes().with_checks(false) {
679714
if let Ok(Attribute { key: k, value: v }) = a {
680715
if let Some(prefix) = k.as_namespace_binding() {
716+
if count >= self.max_declarations_per_element {
717+
return Err(NamespaceError::TooManyDeclarations(
718+
self.max_declarations_per_element,
719+
));
720+
}
721+
count += 1;
681722
self.add(prefix, Namespace(&v))?;
682723
}
683724
} else {
@@ -687,6 +728,32 @@ impl NamespaceResolver {
687728
Ok(())
688729
}
689730

731+
/// Returns the maximum number of `xmlns` / `xmlns:*` declarations that
732+
/// [`push`](Self::push) will accept on a single start tag before returning
733+
/// [`NamespaceError::TooManyDeclarations`].
734+
///
735+
/// Defaults to [`DEFAULT_MAX_DECLARATIONS_PER_ELEMENT`].
736+
#[inline]
737+
pub const fn max_declarations_per_element(&self) -> usize {
738+
self.max_declarations_per_element
739+
}
740+
741+
/// Sets the maximum number of `xmlns` / `xmlns:*` declarations that
742+
/// [`push`](Self::push) will accept on a single start tag.
743+
///
744+
/// `push` is called by [`NsReader`](crate::reader::NsReader) for every
745+
/// `Start`/`Empty` event *before* the event is returned to the caller, so
746+
/// without this limit a start tag with many `xmlns:*` attributes drives
747+
/// unbounded heap allocation that the caller cannot intercept. See
748+
/// <https://github.com/tafia/quick-xml/issues/970>.
749+
///
750+
/// Pass `usize::MAX` to disable the limit.
751+
#[inline]
752+
pub fn set_max_declarations_per_element(&mut self, limit: usize) -> &mut Self {
753+
self.max_declarations_per_element = limit;
754+
self
755+
}
756+
690757
/// Ends a top-most scope by popping all [namespace bindings], that was added by
691758
/// last call to [`Self::push()`] and [`Self::add()`].
692759
///
@@ -1191,6 +1258,52 @@ mod namespaces {
11911258
use pretty_assertions::assert_eq;
11921259
use ResolveResult::*;
11931260

1261+
/// Regression test for <https://github.com/tafia/quick-xml/issues/970>:
1262+
/// `push()` previously allocated one `NamespaceBinding` per `xmlns:*`
1263+
/// attribute with no upper bound, before the caller ever sees the event.
1264+
#[test]
1265+
fn push_rejects_too_many_declarations() {
1266+
let mut tag = String::from("e");
1267+
for i in 0..=DEFAULT_MAX_DECLARATIONS_PER_ELEMENT {
1268+
tag.push_str(&format!(" xmlns:p{}=''", i));
1269+
}
1270+
let mut resolver = NamespaceResolver::default();
1271+
assert_eq!(
1272+
resolver.push(&BytesStart::from_content(&tag, 1)),
1273+
Err(NamespaceError::TooManyDeclarations(
1274+
DEFAULT_MAX_DECLARATIONS_PER_ELEMENT
1275+
)),
1276+
);
1277+
1278+
// Exactly at the limit is accepted.
1279+
let mut tag = String::from("e");
1280+
for i in 0..DEFAULT_MAX_DECLARATIONS_PER_ELEMENT {
1281+
tag.push_str(&format!(" xmlns:p{}=''", i));
1282+
}
1283+
let mut resolver = NamespaceResolver::default();
1284+
assert_eq!(resolver.push(&BytesStart::from_content(&tag, 1)), Ok(()));
1285+
1286+
// The limit is configurable, and `usize::MAX` disables it.
1287+
let mut resolver = NamespaceResolver::default();
1288+
resolver.set_max_declarations_per_element(2);
1289+
assert_eq!(
1290+
resolver.push(&BytesStart::from_content(
1291+
"e xmlns:a='' xmlns:b='' xmlns:c=''",
1292+
1,
1293+
)),
1294+
Err(NamespaceError::TooManyDeclarations(2)),
1295+
);
1296+
let mut resolver = NamespaceResolver::default();
1297+
resolver.set_max_declarations_per_element(usize::MAX);
1298+
assert_eq!(
1299+
resolver.push(&BytesStart::from_content(
1300+
"e xmlns:a='' xmlns:b='' xmlns:c=''",
1301+
1,
1302+
)),
1303+
Ok(()),
1304+
);
1305+
}
1306+
11941307
/// Unprefixed attribute names (resolved with `false` flag) never have a namespace
11951308
/// according to <https://www.w3.org/TR/xml-names11/#defaulting>:
11961309
///

src/reader/ns_reader.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,16 @@ impl<R> NsReader<R> {
121121
pub const fn resolver(&self) -> &NamespaceResolver {
122122
&self.ns_resolver
123123
}
124+
125+
/// Returns a mutable reference to the storage of namespace bindings
126+
/// associated with this reader.
127+
///
128+
/// Useful for configuring the resolver, e.g. to change the
129+
/// [per-element namespace-declaration limit](NamespaceResolver::set_max_declarations_per_element).
130+
#[inline]
131+
pub fn resolver_mut(&mut self) -> &mut NamespaceResolver {
132+
&mut self.ns_resolver
133+
}
124134
}
125135

126136
impl<R: BufRead> NsReader<R> {

0 commit comments

Comments
 (0)