Title: [222005] trunk
- Revision
- 222005
- Author
- wenson_hs...@apple.com
- Date
- 2017-09-13 18:49:05 -0700 (Wed, 13 Sep 2017)
Log Message
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: trunk/LayoutTests/ChangeLog (222004 => 222005)
--- trunk/LayoutTests/ChangeLog 2017-09-14 01:44:37 UTC (rev 222004)
+++ trunk/LayoutTests/ChangeLog 2017-09-14 01:49:05 UTC (rev 222005)
@@ -1,3 +1,16 @@
+2017-09-13 Wenson Hsieh <wenson_hs...@apple.com>
+
+ 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-13 Devin Rousso <web...@devinrousso.com>
Web Inspector: Event Listeners section does not update when listeners are added/removed
Added: trunk/LayoutTests/fast/forms/append-children-during-form-submission-expected.txt (0 => 222005)
--- trunk/LayoutTests/fast/forms/append-children-during-form-submission-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/forms/append-children-during-form-submission-expected.txt 2017-09-14 01:49:05 UTC (rev 222005)
@@ -0,0 +1,2 @@
+
+To manually test, load this page. The web process should not crash.
Added: trunk/LayoutTests/fast/forms/append-children-during-form-submission.html (0 => 222005)
--- trunk/LayoutTests/fast/forms/append-children-during-form-submission.html (rev 0)
+++ trunk/LayoutTests/fast/forms/append-children-during-form-submission.html 2017-09-14 01:49:05 UTC (rev 222005)
@@ -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: trunk/Source/WebCore/ChangeLog (222004 => 222005)
--- trunk/Source/WebCore/ChangeLog 2017-09-14 01:44:37 UTC (rev 222004)
+++ trunk/Source/WebCore/ChangeLog 2017-09-14 01:49:05 UTC (rev 222005)
@@ -1,3 +1,31 @@
+2017-09-13 Wenson Hsieh <wenson_hs...@apple.com>
+
+ 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 Devin Rousso <web...@devinrousso.com>
Web Inspector: Event Listeners section does not update when listeners are added/removed
Modified: trunk/Source/WebCore/loader/FormSubmission.cpp (222004 => 222005)
--- trunk/Source/WebCore/loader/FormSubmission.cpp 2017-09-14 01:44:37 UTC (rev 222004)
+++ trunk/Source/WebCore/loader/FormSubmission.cpp 2017-09-14 01:49:05 UTC (rev 222005)
@@ -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
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
