Skip to content

GitLab

T
tangled
Commit d1622627 authored by Mason Freed's avatar Mason Freed Committed by Commit Bot
Browse files
Options

Fix parser mXSS sanitizer bypass for <p> and <br> within foreign context 

Prior to this CL, the following code:
 <svg></p></svg>
parsed to this innerHTML: <svg><p></p></svg>

This is in contrast to this code:
 <svg><p></svg>
which parses to <svg></svg><p></p>

The fact that the </p> is left inside the <svg> allowed sanitizer
bypasses as detailed in [1]. Please also see [2] for the spec
discussion.

With this CL, </p> and </br> within a foreign context now cause
the closing of the foreign context.

[1] https://research.securitum.com/dompurify-bypass-using-mxss/
[2] https://github.com/whatwg/html/issues/5113

Bug: 1005713
Change-Id: Ic07ee50de4eb1ef19b73a075bd83785c99f4f891
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1940722Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720315}
parent 373d764f
Show whitespace changes
Inline Side-by-side
Showing with 43 additions and 1 deletion
third_party/blink/renderer/core/html/parser/html_tree_builder.cc
View file @ d1622627
...@@ -2813,6 +2813,13 @@ void HTMLTreeBuilder::ProcessTokenInForeignContent(AtomicHTMLToken* token) { ...@@ -2813,6 +2813,13 @@ void HTMLTreeBuilder::ProcessTokenInForeignContent(AtomicHTMLToken* token) {
tree_.OpenElements()->Pop(); tree_.OpenElements()->Pop();
return; return;
} }
if (token->GetName() == html_names::kBrTag ||
token->GetName() == html_names::kPTag) {
ParseError(token);
tree_.OpenElements()->PopUntilForeignContentScopeMarker();
ProcessEndTag(token);
return;
}
if (!tree_.CurrentStackItem()->IsInHTMLNamespace()) { if (!tree_.CurrentStackItem()->IsInHTMLNamespace()) {
// FIXME: This code just wants an Element* iterator, instead of an // FIXME: This code just wants an Element* iterator, instead of an
// ElementRecord* // ElementRecord*
......
third_party/blink/renderer/core/html/parser/html_tree_builder_simulator.cc
View file @ d1622627
...@@ -209,7 +209,13 @@ HTMLTreeBuilderSimulator::SimulatedToken HTMLTreeBuilderSimulator::Simulate( ...@@ -209,7 +209,13 @@ HTMLTreeBuilderSimulator::SimulatedToken HTMLTreeBuilderSimulator::Simulate(
} }
} }
} }
if (token.GetType() == HTMLToken::kEndTag && InForeignContent()) {
const String& tag_name = token.Data();
if (ThreadSafeMatch(tag_name, html_names::kPTag) ||
ThreadSafeMatch(tag_name, html_names::kBrTag)) {
namespace_stack_.pop_back();
}
}
if (token.GetType() == HTMLToken::kEndTag || if (token.GetType() == HTMLToken::kEndTag ||
(token.GetType() == HTMLToken::kStartTag && token.SelfClosing() && (token.GetType() == HTMLToken::kStartTag && token.SelfClosing() &&
InForeignContent())) { InForeignContent())) {
......
third_party/blink/web_tests/external/wpt/html/syntax/parsing/html_content_in_foreign_context.html 0 → 100644
View file @ d1622627
<!DOCTYPE html>
<title>Foreign contexts with HTML tag children</title>
<link rel="author" title="Mason Freed" href="mailto:masonfreed@chromium.org">
<link rel="help" href="https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inbody">
<link rel="help" href="https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inforeign">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
test(function() {
const contexts = ["svg", "math"];
const elements = ["/p", "/br", "b", "big", "blockquote", "br", "center", "code", "dd", "div", "dl", "dt", "em", "embed", "h1", "h2", "h3", "h4", "h5", "h6", "hr", "i", "img", "li", "listing", "menu", "meta", "nobr", "ol", "p", "pre", "ruby", "s", "small", "span", "strong", "strike", "sub", "sup", "table", "tt", "u", "ul", "var"];
contexts.forEach(c => {
elements.forEach(e => {
const wrapper = document.createElement('div');
const html = `<${c}><${e}></${c}`
wrapper.innerHTML = html;
assert_false(wrapper.innerHTML == html, "The inner HTML should get mutated");
const tagname = e[0]=='/' ? e.substr(1) : e;
const element = wrapper.getElementsByTagName(tagname)[0];
assert_true(element !== undefined,`Unable to locate the ${e} node in ${c}`)
const parent = element.parentNode
assert_true(element.parentNode === wrapper,`The ${e} tag did not exit the ${c}`)
});
});
}, "HTML namespace nodes should exit foreign contexts");
</script>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment