Title: [223367] releases/WebKitGTK/webkit-2.18
Revision
223367
Author
[email protected]
Date
2017-10-16 03:25:32 -0700 (Mon, 16 Oct 2017)

Log Message

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
<rdar://problem/34254998>

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
Ref<FormAssociatedElement>s 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.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.18/LayoutTests/ChangeLog (223366 => 223367)


--- releases/WebKitGTK/webkit-2.18/LayoutTests/ChangeLog	2017-10-16 10:21:25 UTC (rev 223366)
+++ releases/WebKitGTK/webkit-2.18/LayoutTests/ChangeLog	2017-10-16 10:25:32 UTC (rev 223367)
@@ -1,3 +1,16 @@
+2017-09-13  Wenson Hsieh  <[email protected]>
+
+        Submitting a form can cause HTMLFormElement's associated elements vector to be mutated during iteration
+        https://bugs.webkit.org/show_bug.cgi?id=176368
+        <rdar://problem/34254998>
+
+        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  <[email protected]>
 
         ASSERTION FAILED: !m_valueOrException under FontFaceSet::completedLoading loading a Serious Eats page

Added: releases/WebKitGTK/webkit-2.18/LayoutTests/fast/forms/append-children-during-form-submission-expected.txt (0 => 223367)


--- releases/WebKitGTK/webkit-2.18/LayoutTests/fast/forms/append-children-during-form-submission-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.18/LayoutTests/fast/forms/append-children-during-form-submission-expected.txt	2017-10-16 10:25:32 UTC (rev 223367)
@@ -0,0 +1,2 @@
+     
+To manually test, load this page. The web process should not crash.

Added: releases/WebKitGTK/webkit-2.18/LayoutTests/fast/forms/append-children-during-form-submission.html (0 => 223367)


--- releases/WebKitGTK/webkit-2.18/LayoutTests/fast/forms/append-children-during-form-submission.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.18/LayoutTests/fast/forms/append-children-during-form-submission.html	2017-10-16 10:25:32 UTC (rev 223367)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<script>
+function loaded() {
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
+    area1.setRangeText("foo");
+    area1.name = "foo";
+    area2.autofocus = true;
+    form.insertBefore(area2, form.firstChild);
+    form.submit();
+}
+
+function changed() {
+    for (let i = 0; i < 100; i++)
+        form.appendChild(document.createElement("input"));
+}
+</script>
+
+<body _onload_=loaded()>
+<form id="form" _onchange_=changed()>
+    <textarea id="area1">a</textarea>
+    <object></object>
+    <textarea id="area2">b</textarea>
+</form>
+<p>To manually test, load this page. The web process should not crash.</p>
+</body>
+</html>
\ No newline at end of file

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog (223366 => 223367)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog	2017-10-16 10:21:25 UTC (rev 223366)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog	2017-10-16 10:25:32 UTC (rev 223367)
@@ -1,3 +1,31 @@
+2017-09-13  Wenson Hsieh  <[email protected]>
+
+        Submitting a form can cause HTMLFormElement's associated elements vector to be mutated during iteration
+        https://bugs.webkit.org/show_bug.cgi?id=176368
+        <rdar://problem/34254998>
+
+        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
+        Ref<FormAssociatedElement>s 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  <[email protected]>
 
         [GTK] Fails to build because 'Float32Array' has not been declared in AudioContext.h

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/loader/FormSubmission.cpp (223366 => 223367)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/loader/FormSubmission.cpp	2017-10-16 10:21:25 UTC (rev 223366)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/loader/FormSubmission.cpp	2017-10-16 10:25:32 UTC (rev 223367)
@@ -191,22 +191,22 @@
     StringPairVector formValues;
 
     bool containsPasswordData = false;
-    {
-        NoEventDispatchAssertion noEventDispatchAssertion;
+    auto protectedAssociatedElements = form.associatedElements().map([] (FormAssociatedElement* rawElement) -> Ref<FormAssociatedElement> {
+        return *rawElement;
+    });
 
-        for (auto& control : form.associatedElements()) {
-            auto& element = control->asHTMLElement();
-            if (!element.isDisabledFormControl())
-                control->appendFormData(domFormData, isMultiPartForm);
-            if (is<HTMLInputElement>(element)) {
-                auto& input = downcast<HTMLInputElement>(element);
-                if (input.isTextField()) {
-                    formValues.append({ input.name().string(), input.value() });
-                    input.addSearchResult();
-                }
-                if (input.isPasswordField() && !input.value().isEmpty())
-                    containsPasswordData = true;
+    for (auto& control : protectedAssociatedElements) {
+        auto& element = control->asHTMLElement();
+        if (!element.isDisabledFormControl())
+            control->appendFormData(domFormData, isMultiPartForm);
+        if (is<HTMLInputElement>(element)) {
+            auto& input = downcast<HTMLInputElement>(element);
+            if (input.isTextField()) {
+                formValues.append({ input.name(), input.value() });
+                input.addSearchResult();
             }
+            if (input.isPasswordField() && !input.value().isEmpty())
+                containsPasswordData = true;
         }
     }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to