diff --git a/CHANGELOG.md b/CHANGELOG.md index d67850a..70d5301 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Unreleased +# 4.1.2 + +* fix: unexpected namespace switches after cleanup can cause mXSS + # 4.1.1 * chore: upgrade to [html5ever 0.35][] diff --git a/Cargo.toml b/Cargo.toml index 052eb98..be85d39 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ammonia" -version = "4.1.1" +version = "4.1.2" authors = ["Michael Howell "] description = "HTML Sanitization" keywords = [ "sanitization", "html", "security", "xss" ] diff --git a/src/lib.rs b/src/lib.rs index 79679e1..f3b3c74 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1841,12 +1841,11 @@ impl<'a> Builder<'a> { let parent = node.parent .replace(None).expect("a node in the DOM will have a parent, except the root, which is not processed") .upgrade().expect("a node's parent will be pointed to by its parent (or the root pointer), and will not be dropped"); - if self.clean_node_content(&node) { + if self.clean_node_content(&node) || !self.check_expected_namespace(&parent, &node) { removed.push(node); continue; } - let pass_clean = self.clean_child(&mut node); - let pass = pass_clean && self.check_expected_namespace(&parent, &node); + let pass = self.clean_child(&mut node); if pass { self.adjust_node_attributes(&mut node, &link_rel, self.id_prefix); dom.append(&parent.clone(), NodeOrText::AppendNode(node.clone())); @@ -2053,21 +2052,18 @@ impl<'a> Builder<'a> { matches!( &*parent.local, "mi" | "mo" | "mn" | "ms" | "mtext" | "annotation-xml" - ) + ) && if child.ns == ns!(html) { is_html_tag(&child.local) } else { true } // The only way to switch from svg to mathml/html is with an html integration point } else if parent.ns == ns!(svg) && child.ns != ns!(svg) { // https://html.spec.whatwg.org/#svg-0 matches!(&*parent.local, "foreignObject") + && if child.ns == ns!(html) { is_html_tag(&child.local) } else { true } } else if child.ns == ns!(svg) { is_svg_tag(&child.local) } else if child.ns == ns!(mathml) { is_mathml_tag(&child.local) } else if child.ns == ns!(html) { - (!is_svg_tag(&child.local) && !is_mathml_tag(&child.local)) - || matches!( - &*child.local, - "title" | "style" | "font" | "a" | "script" | "span" - ) + is_html_tag(&child.local) } else { // There are no other supported ways to switch namespace parent.ns == child.ns @@ -2224,6 +2220,14 @@ fn is_url_attr(element: &str, attr: &str) -> bool { || (element == "video" && attr == "poster") } +fn is_html_tag(element: &str) -> bool { + (!is_svg_tag(element) && !is_mathml_tag(element)) + || matches!( + element, + "title" | "style" | "font" | "a" | "script" | "span" + ) +} + /// Given an element name, check if it's SVG fn is_svg_tag(element: &str) -> bool { // https://svgwg.org/svg2-draft/eltindex.html @@ -3630,15 +3634,15 @@ mod test { // https://github.com/cure53/DOMPurify/pull/495 let fragment = r##"<!--"; + let result = Builder::default() + .strip_comments(false) + .add_tags(&["math","mtext","table","mglyph","xmp"]) + .clean(fragment); + assert_eq!( + result.to_string(), + "
" + ); + } + + #[test] fn xml_processing_instruction() { // https://blog.slonser.info/posts/dompurify-node-type-confusion/