Skip to content

Commit

Permalink
[Backport] Fix for CVE-2020-6464
Browse files Browse the repository at this point in the history
Fix customized built-in element constructor behavior

This CL implements two changes:
 1. It fixes the implementation to better match the spec for the
    "create an element for the token" [1] algorithm. Prior to this CL,
    step 7 of that algorithm was skipping directly to step 6 of the
    "create an element" [2] algorithm, skipping over step 5 for
    customized built-in elements. This is now fixed. This case is
    illustrated by the issue and example at [3] and [4]. This becomes
    the first test in customized-built-in-constructor-exceptions.html.

 2. It updates the comments to match the new behavior discussed in [3]
    and the [5] spec PR, which changes the return value in the case
    that a customized built-in element constructor throws an exception.
    With the change above, that is actually already the behavior. So
    this is just a comment change. Two new tests are added to
    customized-built-in-constructor-exceptions.html.

[1] https://html.spec.whatwg.org/multipage/parsing.html#create-an-element-for-the-token
[2] https://dom.spec.whatwg.org/#concept-create-element
[3] whatwg/html#5084
[4] https://crbug.com/1024866
[5] whatwg/dom#797

[email protected]

(cherry picked from commit 7101418f85a0f17e4f9a35dfe3a9acff76340a93)

Bug: 1071059, 1024866
Change-Id: I814c81991eb5e83501304bcb3d2da476743aef52
Cq-Do-Not-Cancel-Tryjobs: true
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Kent Tamura <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#760705}
Reviewed-by: Mason Freed <[email protected]>
Cr-Commit-Position: refs/branch-heads/4044@{#985}
Cr-Branched-From: a6d9daf149a473ceea37f629c41d4527bf2055bd-refs/heads/master@{#737173}
Reviewed-by: Michal Klocek <[email protected]>
  • Loading branch information
Allan Sandfeld Jensen committed May 6, 2020
1 parent eaf11e7 commit d257670
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ HTMLElement* ScriptCustomElementDefinition::HandleCreateElementSyncException(
HTMLElement* ScriptCustomElementDefinition::CreateAutonomousCustomElementSync(
Document& document,
const QualifiedName& tag_name) {
DCHECK(CustomElement::ShouldCreateCustomElement(tag_name)) << tag_name;
if (!script_state_->ContextIsValid())
return CustomElement::CreateFailedElement(document, tag_name);
ScriptState::Scope scope(script_state_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ Element* CustomElement::CreateUncustomizedOrUndefinedElement(

HTMLElement* CustomElement::CreateFailedElement(Document& document,
const QualifiedName& tag_name) {
DCHECK(ShouldCreateCustomElement(tag_name));
CHECK(ShouldCreateCustomElement(tag_name))
<< "HTMLUnknownElement with built-in tag name: " << tag_name;

// "create an element for a token":
// https://html.spec.whatwg.org/C/#create-an-element-for-the-token
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,19 @@ HTMLElement* CustomElementDefinition::CreateElement(
result->SetCustomElementState(CustomElementState::kUndefined);
result->SetIsValue(Descriptor().GetName());

// 5.3. If the synchronous custom elements flag is set, upgrade
// element using definition.
// 5.4. Otherwise, enqueue a custom element upgrade reaction given
// result and definition.
if (!flags.IsAsyncCustomElements())
if (!flags.IsAsyncCustomElements()) {
// 5.3 If the synchronous custom elements flag is set, then run this step
// while catching any exceptions:
// 1. Upgrade element using definition.
// If this step threw an exception, then:
// 1. Report the exception.
// 2. Set result's custom element state to "failed".
Upgrade(*result);
else
} else {
// 5.4. Otherwise, enqueue a custom element upgrade reaction given
// result and definition.
EnqueueUpgradeReaction(*result);
}
return To<HTMLElement>(result);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -919,8 +919,11 @@ Element* HTMLConstructionSite::CreateElement(
// reactions stack."
CEReactionsScope reactions;

// 7.
element = definition->CreateAutonomousCustomElementSync(document, tag_name);
// 7. Let element be the result of creating an element given document,
// localName, given namespace, null, and is. If will execute script is true,
// set the synchronous custom elements flag; otherwise, leave it unset.
element =
definition->CreateElement(document, tag_name, GetCreateElementFlags());

// "8. Append each attribute in the given token to element." We don't use
// setAttributes here because the custom element constructor may have
Expand Down

0 comments on commit d257670

Please sign in to comment.