Title: [212026] trunk
- Revision
- 212026
- Author
- bfulg...@apple.com
- Date
- 2017-02-09 18:15:02 -0800 (Thu, 09 Feb 2017)
Log Message
Crash under FormSubmission::create()
https://bugs.webkit.org/show_bug.cgi?id=167200
<rdar://problem/30096323>
Patch by Chris Dumez <cdu...@apple.com> on 2017-02-09
Reviewed by Darin Adler.
Source/WebCore:
The issue is that FormSubmission::create() was iterating over
form.associatedElements() as was calling Element::appendFormData()
in the loop. HTMLObjectElement::appendFormData() was calling
pluginWidget(PluginLoadingPolicy::Load) which causes a synchronous
layout and can fire events (such as focus event) synchronously.
Firing those events synchronously allows the JS to modify the
form.associatedElements() vector we are currently iterating on.
To avoid this issue, we now call pluginWidget(PluginLoadingPolicy::DoNotLoad)
in HTMLObjectElement::appendFormData() as we are not allowed to fire
synchronous events at this point. I also added a security assertion
in FormSubmission::create() to catch cases where we fire JS events
while iterating over the form associated elements to more easily
notice these things in the future.
Test: fast/forms/formsubmission-appendFormData-crash.html
* html/HTMLObjectElement.cpp:
(WebCore::HTMLObjectElement::appendFormData):
* loader/FormSubmission.cpp:
(WebCore::FormSubmission::create):
LayoutTests:
Add layout test coverage.
* fast/forms/formsubmission-appendFormData-crash-expected.txt: Added.
* fast/forms/formsubmission-appendFormData-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (212025 => 212026)
--- trunk/LayoutTests/ChangeLog 2017-02-10 02:13:07 UTC (rev 212025)
+++ trunk/LayoutTests/ChangeLog 2017-02-10 02:15:02 UTC (rev 212026)
@@ -1,5 +1,18 @@
2017-02-09 Chris Dumez <cdu...@apple.com>
+ Crash under FormSubmission::create()
+ https://bugs.webkit.org/show_bug.cgi?id=167200
+ <rdar://problem/30096323>
+
+ Reviewed by Darin Adler.
+
+ Add layout test coverage.
+
+ * fast/forms/formsubmission-appendFormData-crash-expected.txt: Added.
+ * fast/forms/formsubmission-appendFormData-crash.html: Added.
+
+2017-02-09 Chris Dumez <cdu...@apple.com>
+
Crash under HTMLFormElement::registerFormElement()
https://bugs.webkit.org/show_bug.cgi?id=167162
Added: trunk/LayoutTests/fast/forms/formsubmission-appendFormData-crash-expected.txt (0 => 212026)
--- trunk/LayoutTests/fast/forms/formsubmission-appendFormData-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/forms/formsubmission-appendFormData-crash-expected.txt 2017-02-10 02:15:02 UTC (rev 212026)
@@ -0,0 +1,3 @@
+This test passes if it does not crash.
+
+ a
Added: trunk/LayoutTests/fast/forms/formsubmission-appendFormData-crash.html (0 => 212026)
--- trunk/LayoutTests/fast/forms/formsubmission-appendFormData-crash.html (rev 0)
+++ trunk/LayoutTests/fast/forms/formsubmission-appendFormData-crash.html 2017-02-10 02:15:02 UTC (rev 212026)
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+
+function runTest() {
+ object.name = "foo";
+ input.autofocus = true;
+ output.appendChild(input);
+ form.submit();
+ setTimeout(function() {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+}
+
+function focushandler() {
+ for(var i = 0; i < 100; i++) {
+ var e = document.createElement("input");
+ form.appendChild(e);
+ }
+}
+</script>
+<body _onload_="runTest()">
+ <p>This test passes if it does not crash.</p>
+ <form id="form">
+ <object id="object">
+ <output id="output">a</output>
+ <input id="input" _onfocus_="focushandler()">
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (212025 => 212026)
--- trunk/Source/WebCore/ChangeLog 2017-02-10 02:13:07 UTC (rev 212025)
+++ trunk/Source/WebCore/ChangeLog 2017-02-10 02:15:02 UTC (rev 212026)
@@ -1,5 +1,35 @@
2017-02-09 Chris Dumez <cdu...@apple.com>
+ Crash under FormSubmission::create()
+ https://bugs.webkit.org/show_bug.cgi?id=167200
+ <rdar://problem/30096323>
+
+ Reviewed by Darin Adler.
+
+ The issue is that FormSubmission::create() was iterating over
+ form.associatedElements() as was calling Element::appendFormData()
+ in the loop. HTMLObjectElement::appendFormData() was calling
+ pluginWidget(PluginLoadingPolicy::Load) which causes a synchronous
+ layout and can fire events (such as focus event) synchronously.
+ Firing those events synchronously allows the JS to modify the
+ form.associatedElements() vector we are currently iterating on.
+
+ To avoid this issue, we now call pluginWidget(PluginLoadingPolicy::DoNotLoad)
+ in HTMLObjectElement::appendFormData() as we are not allowed to fire
+ synchronous events at this point. I also added a security assertion
+ in FormSubmission::create() to catch cases where we fire JS events
+ while iterating over the form associated elements to more easily
+ notice these things in the future.
+
+ Test: fast/forms/formsubmission-appendFormData-crash.html
+
+ * html/HTMLObjectElement.cpp:
+ (WebCore::HTMLObjectElement::appendFormData):
+ * loader/FormSubmission.cpp:
+ (WebCore::FormSubmission::create):
+
+2017-02-09 Chris Dumez <cdu...@apple.com>
+
Crash under HTMLFormElement::registerFormElement()
https://bugs.webkit.org/show_bug.cgi?id=167162
Modified: trunk/Source/WebCore/html/HTMLObjectElement.cpp (212025 => 212026)
--- trunk/Source/WebCore/html/HTMLObjectElement.cpp 2017-02-10 02:13:07 UTC (rev 212025)
+++ trunk/Source/WebCore/html/HTMLObjectElement.cpp 2017-02-10 02:15:02 UTC (rev 212026)
@@ -506,7 +506,9 @@
if (name().isEmpty())
return false;
- Widget* widget = pluginWidget();
+ // Use PluginLoadingPolicy::DoNotLoad here or it would fire JS events synchronously
+ // which would not be safe here.
+ auto* widget = pluginWidget(PluginLoadingPolicy::DoNotLoad);
if (!is<PluginViewBase>(widget))
return false;
String value;
Modified: trunk/Source/WebCore/loader/FormSubmission.cpp (212025 => 212026)
--- trunk/Source/WebCore/loader/FormSubmission.cpp 2017-02-10 02:13:07 UTC (rev 212025)
+++ trunk/Source/WebCore/loader/FormSubmission.cpp 2017-02-10 02:15:02 UTC (rev 212026)
@@ -47,6 +47,7 @@
#include "HTMLInputElement.h"
#include "HTMLNames.h"
#include "HTMLParserIdioms.h"
+#include "NoEventDispatchAssertion.h"
#include "TextEncoding.h"
#include <wtf/CurrentTime.h>
@@ -190,18 +191,22 @@
StringPairVector formValues;
bool containsPasswordData = false;
- 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();
+ {
+ NoEventDispatchAssertion noEventDispatchAssertion;
+
+ 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;
}
- 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
