Skip to content

Commit

Permalink
Change declarative Shadow DOM fragment parsing to be opt-in
Browse files Browse the repository at this point in the history
This CL implements most of the suggestions from [1], which effectively
block declarative Shadow DOM from being used by any fragment parser
entry point, unless an explicit opt-in is toggled.

The opt-ins include:
 - DOMParser.allowDeclarativeShadowDom = true;
 - HTMLTemplateElement.allowDeclarativeShadowDom = true;
 - XMLHttpRequest.allowDeclarativeShadowDom = true;
 - DocumentFragment.allowDeclarativeShadowDom = true;
 - Document.allowDeclarativeShadowDom = true; // For innerHTML
 - A new <iframe> sandbox flag: allow-declarative-shadow-dom

This mitigates the potential client-side XSS sanitizer bypass detailed
in the explainer and at [1]. Assuming these changes are functional,
and mitigate the issue, this new behavior will be folded into the
spec PRs at [2] and [3]. But given the security implications of the
existing code, I'd like to get this landed first.

[1] whatwg/dom#912 (comment)
[2] whatwg/html#5465
[3] whatwg/dom#892

Bug: 1042130
Change-Id: I088f28f63078a0d26e354a4442494c0132b47ffc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2513525
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824591}
GitOrigin-RevId: 83080e44d00b0a9321b54bcca7bbe0eb2f9d5e43
  • Loading branch information
mfreed7 authored and copybara-github committed Nov 5, 2020
1 parent a682511 commit 00903b5
Show file tree
Hide file tree
Showing 31 changed files with 451 additions and 23 deletions.
1 change: 1 addition & 0 deletions blink/common/feature_policy/feature_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ mojom::FeaturePolicyFeature FeaturePolicy::FeatureForSandboxFlag(
kPropagatesToAuxiliaryBrowsingContexts:
case network::mojom::WebSandboxFlags::kTopNavigationByUserActivation:
case network::mojom::WebSandboxFlags::kStorageAccessByUserActivation:
case network::mojom::WebSandboxFlags::kDeclarativeShadowDom:
break;
}
return mojom::FeaturePolicyFeature::kNotFound;
Expand Down
14 changes: 14 additions & 0 deletions blink/renderer/core/dom/document.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6960,6 +6960,20 @@ void Document::MarkFirstPaint() {
MaybeExecuteDelayedAsyncScripts();
}

using AllowState = blink::Document::DeclarativeShadowDomAllowState;
AllowState Document::GetDeclarativeShadowDomAllowState() const {
return declarative_shadow_dom_allow_state_;
}

void Document::setAllowDeclarativeShadowDom(bool val) {
declarative_shadow_dom_allow_state_ =
val ? AllowState::kAllow : AllowState::kDeny;
}

bool Document::allowDeclarativeShadowDom() const {
return declarative_shadow_dom_allow_state_ == AllowState::kAllow;
}

void Document::FinishedParsing() {
DCHECK(!GetScriptableDocumentParser() || !parser_->IsParsing());
DCHECK(!GetScriptableDocumentParser() || ready_state_ != kLoading);
Expand Down
8 changes: 8 additions & 0 deletions blink/renderer/core/dom/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -1662,6 +1662,11 @@ class CORE_EXPORT Document : public ContainerNode,
void MarkFirstPaint();
void MaybeExecuteDelayedAsyncScripts();

enum class DeclarativeShadowDomAllowState { kNotSet, kAllow, kDeny };
DeclarativeShadowDomAllowState GetDeclarativeShadowDomAllowState() const;
void setAllowDeclarativeShadowDom(bool val);
bool allowDeclarativeShadowDom() const;

void SetFindInPageActiveMatchNode(Node*);
const Node* GetFindInPageActiveMatchNode() const;

Expand Down Expand Up @@ -2201,6 +2206,9 @@ class CORE_EXPORT Document : public ContainerNode,
int async_script_count_ = 0;
bool first_paint_recorded_ = false;

DeclarativeShadowDomAllowState declarative_shadow_dom_allow_state_ =
DeclarativeShadowDomAllowState::kNotSet;

WeakMember<Node> find_in_page_active_match_node_;

Member<DocumentData> data_;
Expand Down
3 changes: 3 additions & 0 deletions blink/renderer/core/dom/document.idl
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ typedef (HTMLScriptElement or SVGScriptElement) HTMLOrSVGScriptElement;
// https://w3c.github.io/webappsec-feature-policy/#the-policy-object
readonly attribute FeaturePolicy featurePolicy;

// Declarative Shadow DOM API
[RuntimeEnabled=DeclarativeShadowDOM] attribute boolean allowDeclarativeShadowDom;

// Deprecated prefixed page visibility API.
// TODO(davidben): This is a property so attaching a deprecation warning results in false positives when outputting
// document in the console. It's possible https://crbug.com/43394 will resolve this.
Expand Down
14 changes: 12 additions & 2 deletions blink/renderer/core/dom/document_fragment.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/dom/container_node.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/parser_content_policy.h"
#include "third_party/blink/renderer/platform/wtf/casting.h"

Expand All @@ -49,6 +50,16 @@ class CORE_EXPORT DocumentFragment : public ContainerNode {
bool CanContainRangeEndPoint() const final { return true; }
virtual bool IsTemplateContent() const { return false; }

bool allowDeclarativeShadowDom() const {
return allow_declarative_shadow_dom_;
}
void setAllowDeclarativeShadowDom(bool value) {
allow_declarative_shadow_dom_ = value;
}

// This will catch anyone doing an unnecessary check.
bool IsDocumentFragment() const = delete;

protected:
String nodeName() const final;

Expand All @@ -57,8 +68,7 @@ class CORE_EXPORT DocumentFragment : public ContainerNode {
Node* Clone(Document&, CloneChildrenFlag) const override;
bool ChildTypeAllowed(NodeType) const override;

bool IsDocumentFragment() const =
delete; // This will catch anyone doing an unnecessary check.
bool allow_declarative_shadow_dom_{false};
};

template <>
Expand Down
2 changes: 2 additions & 0 deletions blink/renderer/core/dom/document_fragment.idl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
Exposed=Window
] interface DocumentFragment : Node {
[CallWith=Document] constructor();

[RuntimeEnabled=DeclarativeShadowDOM] attribute boolean allowDeclarativeShadowDom;
};

DocumentFragment includes ParentNode;
Expand Down
1 change: 1 addition & 0 deletions blink/renderer/core/dom/slot_assignment_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class SlotAssignmentTest : public testing::Test {
void SlotAssignmentTest::SetUp() {
dummy_page_holder_ = std::make_unique<DummyPageHolder>(IntSize(800, 600));
document_ = &dummy_page_holder_->GetDocument();
document_->setAllowDeclarativeShadowDom(true);
DCHECK(document_);
}

Expand Down
10 changes: 8 additions & 2 deletions blink/renderer/core/editing/serializers/serialization.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
#include "third_party/blink/renderer/core/html/html_span_element.h"
#include "third_party/blink/renderer/core/html/html_table_cell_element.h"
#include "third_party/blink/renderer/core/html/html_table_element.h"
#include "third_party/blink/renderer/core/html/html_template_element.h"
#include "third_party/blink/renderer/core/html_names.h"
#include "third_party/blink/renderer/core/layout/layout_object.h"
#include "third_party/blink/renderer/core/loader/empty_clients.h"
Expand Down Expand Up @@ -610,8 +611,9 @@ DocumentFragment* CreateFragmentForInnerOuterHTML(
const char* method,
ExceptionState& exception_state) {
DCHECK(context_element);
if (IsA<HTMLTemplateElement>(*context_element) &&
!context_element->GetExecutionContext()) {
const HTMLTemplateElement* template_element =
DynamicTo<HTMLTemplateElement>(*context_element);
if (template_element && !template_element->GetExecutionContext()) {
return nullptr;
}

Expand All @@ -620,6 +622,10 @@ DocumentFragment* CreateFragmentForInnerOuterHTML(
? context_element->GetDocument().EnsureTemplateDocument()
: context_element->GetDocument();
DocumentFragment* fragment = DocumentFragment::Create(document);
fragment->setAllowDeclarativeShadowDom(
context_element->GetDocument().allowDeclarativeShadowDom() ||
(template_element && template_element->content() &&
template_element->content()->allowDeclarativeShadowDom()));

if (IsA<HTMLDocument>(document)) {
fragment->ParseHTML(markup, context_element, parser_content_policy);
Expand Down
10 changes: 6 additions & 4 deletions blink/renderer/core/html/html_iframe_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,12 @@ void HTMLIFrameElement::ParseAttribute(
network::mojom::blink::WebSandboxFlags::kNone;
if (!value.IsNull()) {
using network::mojom::blink::WebSandboxFlags;
WebSandboxFlags ignored_flags =
!RuntimeEnabledFeatures::StorageAccessAPIEnabled()
? WebSandboxFlags::kStorageAccessByUserActivation
: WebSandboxFlags::kNone;
WebSandboxFlags ignored_flags = WebSandboxFlags::kNone;
if (!RuntimeEnabledFeatures::StorageAccessAPIEnabled())
ignored_flags |= WebSandboxFlags::kStorageAccessByUserActivation;
if (!RuntimeEnabledFeatures::DeclarativeShadowDOMEnabled(
GetExecutionContext()))
ignored_flags |= WebSandboxFlags::kDeclarativeShadowDom;

auto parsed = network::ParseWebSandboxPolicy(sandbox_->value().Utf8(),
ignored_flags);
Expand Down
13 changes: 13 additions & 0 deletions blink/renderer/core/html/html_iframe_element_sandbox.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ const char* const kSupportedSandboxTokens[] = {
constexpr char kStorageAccessAPISandboxToken[] =
"allow-storage-access-by-user-activation";

// TODO(crbug.com/1042130): move this into |kSupportedSandboxTokens| when the
// feature flag is enabled by default.
constexpr char kDeclarativeShadowDom[] = "allow-declarative-shadow-dom";

bool IsTokenSupported(const AtomicString& token) {
for (const char* supported_token : kSupportedSandboxTokens) {
if (token == supported_token)
Expand All @@ -46,6 +50,15 @@ bool IsTokenSupported(const AtomicString& token) {
(token == kStorageAccessAPISandboxToken)) {
return true;
}

// If Declarative Shadow DOM is enabled, allow the sandbox flag.
// TODO(crbug.com/1145605): This won't work for origin trial enabled iframe
// documents, because there's no ExecutionContext here.
if (RuntimeEnabledFeatures::DeclarativeShadowDOMEnabledByRuntimeFlag() &&
(token == kDeclarativeShadowDom)) {
return true;
}

return false;
}

Expand Down
7 changes: 6 additions & 1 deletion blink/renderer/core/html/parser/html_construction_site.cc
Original file line number Diff line number Diff line change
Expand Up @@ -713,10 +713,15 @@ void HTMLConstructionSite::InsertHTMLFormElement(AtomicHTMLToken* token,

void HTMLConstructionSite::InsertHTMLTemplateElement(
AtomicHTMLToken* token,
DeclarativeShadowRootType declarative_shadow_root_type) {
DeclarativeShadowRootType declarative_shadow_root_type,
bool allow_declarative_shadow_dom) {
auto* template_element = To<HTMLTemplateElement>(
CreateElement(token, html_names::xhtmlNamespaceURI));
template_element->SetDeclarativeShadowRootType(declarative_shadow_root_type);
if (DocumentFragment* content =
template_element->TemplateContentForHTMLConstructionSite()) {
content->setAllowDeclarativeShadowDom(allow_declarative_shadow_dom);
}
AttachLater(CurrentNode(), template_element);
open_elements_.Push(
MakeGarbageCollected<HTMLStackItem>(template_element, token));
Expand Down
6 changes: 3 additions & 3 deletions blink/renderer/core/html/parser/html_construction_site.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,9 @@ class HTMLConstructionSite final {
void InsertCommentOnDocument(AtomicHTMLToken*);
void InsertCommentOnHTMLHtmlElement(AtomicHTMLToken*);
void InsertHTMLElement(AtomicHTMLToken*);
void InsertHTMLTemplateElement(
AtomicHTMLToken*,
DeclarativeShadowRootType declarative_shadow_root_type);
void InsertHTMLTemplateElement(AtomicHTMLToken*,
DeclarativeShadowRootType,
bool allow_declarative_shadow_dom);
void InsertSelfClosingHTMLElementDestroyingToken(AtomicHTMLToken*);
void InsertFormattingElement(AtomicHTMLToken*);
void InsertHTMLHeadElement(AtomicHTMLToken*);
Expand Down
27 changes: 25 additions & 2 deletions blink/renderer/core/html/parser/html_document_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,30 @@ HTMLDocumentParser::HTMLDocumentParser(HTMLDocument& document,
: HTMLDocumentParser(document, kAllowScriptingContent, sync_policy) {
script_runner_ =
HTMLParserScriptRunner::Create(ReentryPermit(), &document, this);

// Deny declarative Shadow DOM if explicitly denied.
// Allow declarative shadow DOM:
// 1. For the main frame (non-fragment) document, or
// 2. When explicitly enabled by the allowDeclarativeShadowDom flag, or
// 3. For sub-frames that do not have this feature sandboxed.
bool allow_declarative_shadow_dom = false;
if (document.GetDeclarativeShadowDomAllowState() !=
Document::DeclarativeShadowDomAllowState::kDeny) {
bool is_main_frame =
document.GetFrame() && document.GetFrame()->IsMainFrame();
const auto* context = document.GetExecutionContext();
allow_declarative_shadow_dom =
is_main_frame ||
(document.GetDeclarativeShadowDomAllowState() ==
Document::DeclarativeShadowDomAllowState::kAllow) ||
(!is_main_frame && context &&
!context->IsSandboxed(
network::mojom::blink::WebSandboxFlags::kDeclarativeShadowDom));
}

tree_builder_ = MakeGarbageCollected<HTMLTreeBuilder>(
this, document, kAllowScriptingContent, options_);
this, document, kAllowScriptingContent, options_,
allow_declarative_shadow_dom);
}

HTMLDocumentParser::HTMLDocumentParser(
Expand All @@ -300,7 +322,8 @@ HTMLDocumentParser::HTMLDocumentParser(
kForceSynchronousParsing) {
// No script_runner_ in fragment parser.
tree_builder_ = MakeGarbageCollected<HTMLTreeBuilder>(
this, fragment, context_element, parser_content_policy, options_);
this, fragment, context_element, parser_content_policy, options_,
fragment->allowDeclarativeShadowDom());

// For now document fragment parsing never reports errors.
bool report_errors = false;
Expand Down
16 changes: 11 additions & 5 deletions blink/renderer/core/html/parser/html_tree_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -232,12 +232,14 @@ class HTMLTreeBuilder::CharacterTokenBuffer {
HTMLTreeBuilder::HTMLTreeBuilder(HTMLDocumentParser* parser,
Document& document,
ParserContentPolicy parser_content_policy,
const HTMLParserOptions& options)
const HTMLParserOptions& options,
bool allow_declarative_shadow_dom)
: frameset_ok_(true),
tree_(parser->ReentryPermit(), document, parser_content_policy),
insertion_mode_(kInitialMode),
original_insertion_mode_(kInitialMode),
should_skip_leading_newline_(false),
allow_declarative_shadow_dom_(allow_declarative_shadow_dom),
parser_(parser),
script_to_process_start_position_(UninitializedPositionValue1()),
options_(options) {}
Expand All @@ -246,11 +248,13 @@ HTMLTreeBuilder::HTMLTreeBuilder(HTMLDocumentParser* parser,
DocumentFragment* fragment,
Element* context_element,
ParserContentPolicy parser_content_policy,
const HTMLParserOptions& options)
const HTMLParserOptions& options,
bool allow_declarative_shadow_dom)
: HTMLTreeBuilder(parser,
fragment->GetDocument(),
parser_content_policy,
options) {
options,
allow_declarative_shadow_dom) {
DCHECK(IsMainThread());
DCHECK(context_element);
tree_.InitFragmentParsing(fragment, context_element);
Expand Down Expand Up @@ -900,7 +904,8 @@ void HTMLTreeBuilder::ProcessTemplateStartTag(AtomicHTMLToken* token) {
DeclarativeShadowRootType declarative_shadow_root_type(
DeclarativeShadowRootType::kNone);
if (RuntimeEnabledFeatures::DeclarativeShadowDOMEnabled(
tree_.CurrentNode()->GetExecutionContext())) {
tree_.CurrentNode()->GetExecutionContext()) &&
allow_declarative_shadow_dom_) {
if (Attribute* type_attribute =
token->GetAttributeItem(html_names::kShadowrootAttr)) {
String shadow_mode = type_attribute->Value();
Expand All @@ -919,7 +924,8 @@ void HTMLTreeBuilder::ProcessTemplateStartTag(AtomicHTMLToken* token) {
}
}
}
tree_.InsertHTMLTemplateElement(token, declarative_shadow_root_type);
tree_.InsertHTMLTemplateElement(token, declarative_shadow_root_type,
allow_declarative_shadow_dom_);
frameset_ok_ = false;
template_insertion_modes_.push_back(kTemplateContentsMode);
SetInsertionMode(kTemplateContentsMode);
Expand Down
8 changes: 6 additions & 2 deletions blink/renderer/core/html/parser/html_tree_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,14 @@ class HTMLTreeBuilder final : public GarbageCollected<HTMLTreeBuilder> {
HTMLTreeBuilder(HTMLDocumentParser*,
Document&,
ParserContentPolicy,
const HTMLParserOptions&);
const HTMLParserOptions&,
bool allow_declarative_shadow_dom);
HTMLTreeBuilder(HTMLDocumentParser*,
DocumentFragment*,
Element* context_element,
ParserContentPolicy,
const HTMLParserOptions&);
const HTMLParserOptions&,
bool allow_declarative_shadow_dom);
~HTMLTreeBuilder();
void Trace(Visitor*) const;

Expand Down Expand Up @@ -246,6 +248,8 @@ class HTMLTreeBuilder final : public GarbageCollected<HTMLTreeBuilder> {

bool should_skip_leading_newline_;

const bool allow_declarative_shadow_dom_;

// We access parser because HTML5 spec requires that we be able to change the
// state of the tokenizer from within parser actions. We also need it to track
// the current position.
Expand Down
1 change: 1 addition & 0 deletions blink/renderer/core/xml/dom_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Document* DOMParser::parseFromString(const String& str, const String& type) {
.WithTypeFrom(type)
.WithExecutionContext(window_)
.CreateDocument();
doc->setAllowDeclarativeShadowDom(allow_declarative_shadow_dom_);
doc->SetContent(str);
doc->SetMimeType(AtomicString(type));
return doc;
Expand Down
8 changes: 8 additions & 0 deletions blink/renderer/core/xml/dom_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,19 @@ class DOMParser final : public ScriptWrappable {

Document* parseFromString(const String&, const String& type);

bool allowDeclarativeShadowDom() const {
return allow_declarative_shadow_dom_;
}
void setAllowDeclarativeShadowDom(bool value) {
allow_declarative_shadow_dom_ = value;
}

void Trace(Visitor*) const override;

LocalDOMWindow* GetWindow() const { return window_.Get(); }

private:
bool allow_declarative_shadow_dom_{false};
WeakMember<LocalDOMWindow> window_;
};

Expand Down
1 change: 1 addition & 0 deletions blink/renderer/core/xml/dom_parser.idl
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,5 @@ enum SupportedType {
] interface DOMParser {
[CallWith=ScriptState] constructor();
[NewObject] Document parseFromString(HTMLString str, SupportedType type);
[RuntimeEnabled=DeclarativeShadowDOM] attribute boolean allowDeclarativeShadowDom;
};
2 changes: 2 additions & 0 deletions blink/renderer/core/xmlhttprequest/xml_http_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,8 @@ void XMLHttpRequest::InitResponseDocument() {
// FIXME: Set Last-Modified.
response_document_->SetContextFeatures(document->GetContextFeatures());
response_document_->SetMimeType(FinalResponseMIMETypeWithFallback());
response_document_->setAllowDeclarativeShadowDom(
allow_declarative_shadow_dom_);
}

Document* XMLHttpRequest::responseXML(ExceptionState& exception_state) {
Expand Down
Loading

0 comments on commit 00903b5

Please sign in to comment.