From c460b82046caae3379f339dcd79f006abd4ab0fc Mon Sep 17 00:00:00 2001 From: Wenson Hsieh Date: Mon, 16 Oct 2017 10:25:32 +0000 Subject: [PATCH] Merge r222005 - Submitting a form can cause HTMLFormElement's associated elements vector to be mutated during iteration https://bugs.webkit.org/show_bug.cgi?id=176368 Reviewed by Ryosuke Niwa. Source/WebCore: In the process of iterating over form.associatedElements() during form submission in FormSubmission::create, the page may cause us to clobber the vector of FormAssociatedElements* we're currently iterating over by inserting new form controls beneath the form element we're in the process of submitting. This happens because FormSubmission::create calls HTMLTextAreaElement::appendFormData, which requires layout to be up to date, which in turn makes us updateLayout() and set focus, which fires a `change` event, upon which the page's JavaScript inserts additonal DOM nodes into the form, modifying the vector of associated elements. To mitigate this, instead of iterating over HTMLFormElement::associatedElements(), which returns a reference to the HTMLFormElement's actual m_associatedElements vector, we iterate over a new vector of Refs created from m_associatedElements. This patch also removes an event dispatch assertion added in r212026. This assertion was added to catch any other events dispatched in this scope, since dispatching events there would have had security implications, but after making iteration over associated elements robust, this NoEventDispatchAssertion is no longer useful. Test: fast/forms/append-children-during-form-submission.html * loader/FormSubmission.cpp: (WebCore::FormSubmission::create): LayoutTests: Adds a new test to make sure we don't crash when mutating a form's associated elements during form submission. * fast/forms/append-children-during-form-submission-expected.txt: Added. * fast/forms/append-children-during-form-submission.html: Added. --- LayoutTests/ChangeLog | 13 ++++++++ ...ildren-during-form-submission-expected.txt | 2 ++ ...ppend-children-during-form-submission.html | 29 ++++++++++++++++++ Source/WebCore/ChangeLog | 28 +++++++++++++++++ Source/WebCore/loader/FormSubmission.cpp | 30 +++++++++---------- 5 files changed, 87 insertions(+), 15 deletions(-) create mode 100644 LayoutTests/fast/forms/append-children-during-form-submission-expected.txt create mode 100644 LayoutTests/fast/forms/append-children-during-form-submission.html diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 53f2f7046b55..697bbbd3ee34 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,16 @@ +2017-09-13 Wenson Hsieh + + Submitting a form can cause HTMLFormElement's associated elements vector to be mutated during iteration + https://bugs.webkit.org/show_bug.cgi?id=176368 + + + Reviewed by Ryosuke Niwa. + + Adds a new test to make sure we don't crash when mutating a form's associated elements during form submission. + + * fast/forms/append-children-during-form-submission-expected.txt: Added. + * fast/forms/append-children-during-form-submission.html: Added. + 2017-09-12 Myles C. Maxfield ASSERTION FAILED: !m_valueOrException under FontFaceSet::completedLoading loading a Serious Eats page diff --git a/LayoutTests/fast/forms/append-children-during-form-submission-expected.txt b/LayoutTests/fast/forms/append-children-during-form-submission-expected.txt new file mode 100644 index 000000000000..6020a9f89482 --- /dev/null +++ b/LayoutTests/fast/forms/append-children-during-form-submission-expected.txt @@ -0,0 +1,2 @@ + +To manually test, load this page. The web process should not crash. diff --git a/LayoutTests/fast/forms/append-children-during-form-submission.html b/LayoutTests/fast/forms/append-children-during-form-submission.html new file mode 100644 index 000000000000..fb13bb84705b --- /dev/null +++ b/LayoutTests/fast/forms/append-children-during-form-submission.html @@ -0,0 +1,29 @@ + + + + + +
+ + + +
+

To manually test, load this page. The web process should not crash.

+ + \ No newline at end of file diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 997cac2aa697..0a971d2161a2 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,31 @@ +2017-09-13 Wenson Hsieh + + Submitting a form can cause HTMLFormElement's associated elements vector to be mutated during iteration + https://bugs.webkit.org/show_bug.cgi?id=176368 + + + Reviewed by Ryosuke Niwa. + + In the process of iterating over form.associatedElements() during form submission in FormSubmission::create, the + page may cause us to clobber the vector of FormAssociatedElements* we're currently iterating over by inserting + new form controls beneath the form element we're in the process of submitting. This happens because + FormSubmission::create calls HTMLTextAreaElement::appendFormData, which requires layout to be up to date, which + in turn makes us updateLayout() and set focus, which fires a `change` event, upon which the page's JavaScript + inserts additonal DOM nodes into the form, modifying the vector of associated elements. + + To mitigate this, instead of iterating over HTMLFormElement::associatedElements(), which returns a reference to + the HTMLFormElement's actual m_associatedElements vector, we iterate over a new vector of + Refs created from m_associatedElements. + + This patch also removes an event dispatch assertion added in r212026. This assertion was added to catch any + other events dispatched in this scope, since dispatching events there would have had security implications, but + after making iteration over associated elements robust, this NoEventDispatchAssertion is no longer useful. + + Test: fast/forms/append-children-during-form-submission.html + + * loader/FormSubmission.cpp: + (WebCore::FormSubmission::create): + 2017-09-13 Carlos Alberto Lopez Perez [GTK] Fails to build because 'Float32Array' has not been declared in AudioContext.h diff --git a/Source/WebCore/loader/FormSubmission.cpp b/Source/WebCore/loader/FormSubmission.cpp index 1d742ea334d0..b4688b3139da 100644 --- a/Source/WebCore/loader/FormSubmission.cpp +++ b/Source/WebCore/loader/FormSubmission.cpp @@ -191,22 +191,22 @@ Ref FormSubmission::create(HTMLFormElement& form, const Attribut StringPairVector formValues; bool containsPasswordData = false; - { - NoEventDispatchAssertion noEventDispatchAssertion; - - for (auto& control : form.associatedElements()) { - auto& element = control->asHTMLElement(); - if (!element.isDisabledFormControl()) - control->appendFormData(domFormData, isMultiPartForm); - if (is(element)) { - auto& input = downcast(element); - if (input.isTextField()) { - formValues.append({ input.name().string(), input.value() }); - input.addSearchResult(); - } - if (input.isPasswordField() && !input.value().isEmpty()) - containsPasswordData = true; + auto protectedAssociatedElements = form.associatedElements().map([] (FormAssociatedElement* rawElement) -> Ref { + return *rawElement; + }); + + for (auto& control : protectedAssociatedElements) { + auto& element = control->asHTMLElement(); + if (!element.isDisabledFormControl()) + control->appendFormData(domFormData, isMultiPartForm); + if (is(element)) { + auto& input = downcast(element); + if (input.isTextField()) { + formValues.append({ input.name(), input.value() }); + input.addSearchResult(); } + if (input.isPasswordField() && !input.value().isEmpty()) + containsPasswordData = true; } }