- Revision
- 227479
- Author
- wenson_hs...@apple.com
- Date
- 2018-01-24 00:19:16 -0800 (Wed, 24 Jan 2018)
Log Message
Harden against layout passes triggered when iterating through HTMLFormElement::associatedElements
https://bugs.webkit.org/show_bug.cgi?id=182037
<rdar://problem/36747812>
Reviewed by Ryosuke Niwa.
Source/WebCore:
Observe that HTMLFormElement::associatedElements returns a const reference to a Vector of raw
FormAssociatedElement pointers. In various call sites that iterate through these associated elements using this
function, some require synchronous layout updates per iteration, which can lead to a bad time when combined with
the first observation.
To address this, we introduce HTMLFormElement::copyAssociatedElementsVector. This returns a new vector
containing strong Refs to each associated element. From each call site that may trigger synchronous layout and
execute arbitrary script while iterating over associated form elements, we instead use iterate over protected
FormAssociatedElements.
>From each call site that currently doesn't (and shouldn't) require a layout update, we use the old version that
returns a list of raw FormAssociatedElement pointers, but add ScriptDisallowedScopes to ensure that we never
execute script there in the future.
Test: fast/forms/form-data-associated-element-iteration.html
* html/DOMFormData.cpp:
(WebCore::DOMFormData::DOMFormData):
Change to use copyAssociatedElementsVector().
* html/FormController.cpp:
(WebCore::recordFormStructure):
(WebCore::FormController::restoreControlStateIn):
Change to use copyAssociatedElementsVector().
* html/HTMLFieldSetElement.cpp:
(WebCore::HTMLFieldSetElement::copyAssociatedElementsVector const):
(WebCore:: const):
(WebCore::HTMLFieldSetElement::length const):
Refactor to use unsafeAssociatedElements().
* html/HTMLFieldSetElement.h:
* html/HTMLFormControlsCollection.cpp:
(WebCore:: const):
(WebCore::HTMLFormControlsCollection::copyFormControlElementsVector const):
(WebCore::HTMLFormControlsCollection::customElementAfter const):
(WebCore::HTMLFormControlsCollection::updateNamedElementCache const):
Refactor these to use unsafeAssociatedElements().
* html/HTMLFormControlsCollection.h:
* html/HTMLFormElement.cpp:
(WebCore::HTMLFormElement::unsafeAssociatedElements const):
(WebCore::HTMLFormElement::copyAssociatedElementsVector const):
* html/HTMLFormElement.h:
* loader/FormSubmission.cpp:
(WebCore::FormSubmission::create):
Refactor to use copyAssociatedElementsVector().
Source/WebKitLegacy/mac:
Rename associatedElements() to unsafeAssociatedElements(), and add ScriptDisallowedScopes. See WebCore ChangeLog
for more details.
* WebView/WebHTMLRepresentation.mm:
(-[WebHTMLRepresentation elementWithName:inForm:]):
(-[WebHTMLRepresentation controlsInForm:]):
LayoutTests:
Add a new layout test covering these hardening changes. See WebCore ChangeLog for more details.
* fast/forms/form-data-associated-element-iteration-expected.txt: Added.
* fast/forms/form-data-associated-element-iteration.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (227478 => 227479)
--- trunk/LayoutTests/ChangeLog 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/LayoutTests/ChangeLog 2018-01-24 08:19:16 UTC (rev 227479)
@@ -1,3 +1,16 @@
+2018-01-23 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Harden against layout passes triggered when iterating through HTMLFormElement::associatedElements
+ https://bugs.webkit.org/show_bug.cgi?id=182037
+ <rdar://problem/36747812>
+
+ Reviewed by Ryosuke Niwa.
+
+ Add a new layout test covering these hardening changes. See WebCore ChangeLog for more details.
+
+ * fast/forms/form-data-associated-element-iteration-expected.txt: Added.
+ * fast/forms/form-data-associated-element-iteration.html: Added.
+
2018-01-23 Yusuke Suzuki <utatane....@gmail.com>
Import WPT for modules
Added: trunk/LayoutTests/fast/forms/form-data-associated-element-iteration-expected.txt (0 => 227479)
--- trunk/LayoutTests/fast/forms/form-data-associated-element-iteration-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/forms/form-data-associated-element-iteration-expected.txt 2018-01-24 08:19:16 UTC (rev 227479)
@@ -0,0 +1,2 @@
+This test passes if the web content process does not crash.
+
Added: trunk/LayoutTests/fast/forms/form-data-associated-element-iteration.html (0 => 227479)
--- trunk/LayoutTests/fast/forms/form-data-associated-element-iteration.html (rev 0)
+++ trunk/LayoutTests/fast/forms/form-data-associated-element-iteration.html 2018-01-24 08:19:16 UTC (rev 227479)
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html>
+<form id="form"></form>
+<div>This test passes if the web content process does not crash.</div>
+<script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ textAreaCount = 100;
+ for (let i = 1; i <= textAreaCount; ++i) {
+ let textarea = document.createElement("textarea");
+ textarea.id = textarea.name = `textarea-${i}`;
+ form.appendChild(textarea);
+ }
+
+ layoutCount = 1;
+ frame = document.body.appendChild(document.createElement("iframe"));
+ frame.contentWindow.matchMedia("(max-width: 100px)").addListener(listener);
+ new FormData(form);
+
+ function listener() {
+ if (layoutCount == textAreaCount - 1) {
+ for (let i = 0; i <= textAreaCount; ++i) {
+ let textarea = document.getElementById(`textarea-${i}`);
+ if (!textarea)
+ continue;
+
+ textarea.remove();
+ textarea = null;
+ }
+ }
+
+ if (layoutCount <= textAreaCount) {
+ frame.contentWindow.matchMedia(`(max-width: ${layoutCount + 1}px)`).addListener(listener);
+ frame.width = layoutCount++;
+ }
+ }
+
+ form.remove();
+</script>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (227478 => 227479)
--- trunk/Source/WebCore/ChangeLog 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebCore/ChangeLog 2018-01-24 08:19:16 UTC (rev 227479)
@@ -1,3 +1,64 @@
+2018-01-23 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Harden against layout passes triggered when iterating through HTMLFormElement::associatedElements
+ https://bugs.webkit.org/show_bug.cgi?id=182037
+ <rdar://problem/36747812>
+
+ Reviewed by Ryosuke Niwa.
+
+ Observe that HTMLFormElement::associatedElements returns a const reference to a Vector of raw
+ FormAssociatedElement pointers. In various call sites that iterate through these associated elements using this
+ function, some require synchronous layout updates per iteration, which can lead to a bad time when combined with
+ the first observation.
+
+ To address this, we introduce HTMLFormElement::copyAssociatedElementsVector. This returns a new vector
+ containing strong Refs to each associated element. From each call site that may trigger synchronous layout and
+ execute arbitrary script while iterating over associated form elements, we instead use iterate over protected
+ FormAssociatedElements.
+
+ From each call site that currently doesn't (and shouldn't) require a layout update, we use the old version that
+ returns a list of raw FormAssociatedElement pointers, but add ScriptDisallowedScopes to ensure that we never
+ execute script there in the future.
+
+ Test: fast/forms/form-data-associated-element-iteration.html
+
+ * html/DOMFormData.cpp:
+ (WebCore::DOMFormData::DOMFormData):
+
+ Change to use copyAssociatedElementsVector().
+
+ * html/FormController.cpp:
+ (WebCore::recordFormStructure):
+ (WebCore::FormController::restoreControlStateIn):
+
+ Change to use copyAssociatedElementsVector().
+
+ * html/HTMLFieldSetElement.cpp:
+ (WebCore::HTMLFieldSetElement::copyAssociatedElementsVector const):
+ (WebCore:: const):
+ (WebCore::HTMLFieldSetElement::length const):
+
+ Refactor to use unsafeAssociatedElements().
+
+ * html/HTMLFieldSetElement.h:
+ * html/HTMLFormControlsCollection.cpp:
+ (WebCore:: const):
+ (WebCore::HTMLFormControlsCollection::copyFormControlElementsVector const):
+ (WebCore::HTMLFormControlsCollection::customElementAfter const):
+ (WebCore::HTMLFormControlsCollection::updateNamedElementCache const):
+
+ Refactor these to use unsafeAssociatedElements().
+
+ * html/HTMLFormControlsCollection.h:
+ * html/HTMLFormElement.cpp:
+ (WebCore::HTMLFormElement::unsafeAssociatedElements const):
+ (WebCore::HTMLFormElement::copyAssociatedElementsVector const):
+ * html/HTMLFormElement.h:
+ * loader/FormSubmission.cpp:
+ (WebCore::FormSubmission::create):
+
+ Refactor to use copyAssociatedElementsVector().
+
2018-01-23 Basuke Suzuki <basuke.suz...@sony.com>
[Curl] Fix wrong redirection with relative url when it happens from
Modified: trunk/Source/WebCore/html/DOMFormData.cpp (227478 => 227479)
--- trunk/Source/WebCore/html/DOMFormData.cpp 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebCore/html/DOMFormData.cpp 2018-01-24 08:19:16 UTC (rev 227479)
@@ -47,7 +47,7 @@
if (!form)
return;
- for (auto& element : form->associatedElements()) {
+ for (auto& element : form->copyAssociatedElementsVector()) {
if (!element->asHTMLElement().isDisabledFormControl())
element->appendFormData(*this, true);
}
Modified: trunk/Source/WebCore/html/FormController.cpp (227478 => 227479)
--- trunk/Source/WebCore/html/FormController.cpp 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebCore/html/FormController.cpp 2018-01-24 08:19:16 UTC (rev 227479)
@@ -23,6 +23,7 @@
#include "HTMLFormElement.h"
#include "HTMLInputElement.h"
+#include "ScriptDisallowedScope.h"
#include <wtf/NeverDestroyed.h>
#include <wtf/text/StringBuilder.h>
#include <wtf/text/StringConcatenateNumbers.h>
@@ -279,9 +280,10 @@
static inline void recordFormStructure(const HTMLFormElement& form, StringBuilder& builder)
{
+ ScriptDisallowedScope::InMainThread scriptDisallowedScope;
// 2 is enough to distinguish forms in webkit.org/b/91209#c0
const size_t namedControlsToBeRecorded = 2;
- const Vector<FormAssociatedElement*>& controls = form.associatedElements();
+ auto& controls = form.unsafeAssociatedElements();
builder.appendLiteral(" [");
for (size_t i = 0, namedControls = 0; i < controls.size() && namedControls < namedControlsToBeRecorded; ++i) {
if (!controls[i]->isFormControlElementWithState())
@@ -454,17 +456,17 @@
void FormController::restoreControlStateIn(HTMLFormElement& form)
{
- for (auto& element : form.associatedElements()) {
- if (!is<HTMLFormControlElementWithState>(*element))
+ for (auto& element : form.copyAssociatedElementsVector()) {
+ if (!is<HTMLFormControlElementWithState>(element.get()))
continue;
- auto control = makeRef(downcast<HTMLFormControlElementWithState>(*element));
- if (!control->shouldSaveAndRestoreFormControlState())
+ auto& control = downcast<HTMLFormControlElementWithState>(element.get());
+ if (!control.shouldSaveAndRestoreFormControlState())
continue;
if (ownerFormForState(control) != &form)
continue;
auto state = takeStateForFormElement(control);
if (!state.isEmpty())
- control->restoreFormControlState(state);
+ control.restoreFormControlState(state);
}
}
Modified: trunk/Source/WebCore/html/HTMLFieldSetElement.cpp (227478 => 227479)
--- trunk/Source/WebCore/html/HTMLFieldSetElement.cpp 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebCore/html/HTMLFieldSetElement.cpp 2018-01-24 08:19:16 UTC (rev 227479)
@@ -32,6 +32,7 @@
#include "HTMLObjectElement.h"
#include "NodeRareData.h"
#include "RenderElement.h"
+#include "ScriptDisallowedScope.h"
#include <wtf/StdLibExtras.h>
namespace WebCore {
@@ -191,16 +192,26 @@
}
}
-const Vector<FormAssociatedElement*>& HTMLFieldSetElement::associatedElements() const
+Vector<Ref<FormAssociatedElement>> HTMLFieldSetElement::copyAssociatedElementsVector() const
{
updateAssociatedElements();
+ return WTF::map(m_associatedElements, [] (auto* rawElement) {
+ return Ref<FormAssociatedElement>(*rawElement);
+ });
+}
+
+const Vector<FormAssociatedElement*>& HTMLFieldSetElement::unsafeAssociatedElements() const
+{
+ ASSERT(!ScriptDisallowedScope::InMainThread::isScriptAllowed());
+ updateAssociatedElements();
return m_associatedElements;
}
unsigned HTMLFieldSetElement::length() const
{
+ ScriptDisallowedScope::InMainThread scriptDisallowedScope;
unsigned length = 0;
- for (auto* element : associatedElements()) {
+ for (auto* element : unsafeAssociatedElements()) {
if (element->isEnumeratable())
++length;
}
Modified: trunk/Source/WebCore/html/HTMLFieldSetElement.h (227478 => 227479)
--- trunk/Source/WebCore/html/HTMLFieldSetElement.h 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebCore/html/HTMLFieldSetElement.h 2018-01-24 08:19:16 UTC (rev 227479)
@@ -40,7 +40,8 @@
Ref<HTMLFormControlsCollection> elements();
Ref<HTMLCollection> elementsForNativeBindings();
- const Vector<FormAssociatedElement*>& associatedElements() const;
+ const Vector<FormAssociatedElement*>& unsafeAssociatedElements() const;
+ Vector<Ref<FormAssociatedElement>> copyAssociatedElementsVector() const;
unsigned length() const;
void addInvalidDescendant(const HTMLFormControlElement&);
Modified: trunk/Source/WebCore/html/HTMLFormControlsCollection.cpp (227478 => 227479)
--- trunk/Source/WebCore/html/HTMLFormControlsCollection.cpp 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebCore/html/HTMLFormControlsCollection.cpp 2018-01-24 08:19:16 UTC (rev 227479)
@@ -27,6 +27,7 @@
#include "HTMLFormElement.h"
#include "HTMLImageElement.h"
#include "HTMLNames.h"
+#include "ScriptDisallowedScope.h"
namespace WebCore {
@@ -62,14 +63,22 @@
return Variant<RefPtr<RadioNodeList>, RefPtr<Element>> { RefPtr<RadioNodeList> { ownerNode().radioNodeList(name) } };
}
-const Vector<FormAssociatedElement*>& HTMLFormControlsCollection::formControlElements() const
+const Vector<FormAssociatedElement*>& HTMLFormControlsCollection::unsafeFormControlElements() const
{
ASSERT(is<HTMLFormElement>(ownerNode()) || is<HTMLFieldSetElement>(ownerNode()));
if (is<HTMLFormElement>(ownerNode()))
- return downcast<HTMLFormElement>(ownerNode()).associatedElements();
- return downcast<HTMLFieldSetElement>(ownerNode()).associatedElements();
+ return downcast<HTMLFormElement>(ownerNode()).unsafeAssociatedElements();
+ return downcast<HTMLFieldSetElement>(ownerNode()).unsafeAssociatedElements();
}
+Vector<Ref<FormAssociatedElement>> HTMLFormControlsCollection::copyFormControlElementsVector() const
+{
+ ASSERT(is<HTMLFormElement>(ownerNode()) || is<HTMLFieldSetElement>(ownerNode()));
+ if (is<HTMLFormElement>(ownerNode()))
+ return downcast<HTMLFormElement>(ownerNode()).copyAssociatedElementsVector();
+ return downcast<HTMLFieldSetElement>(ownerNode()).copyAssociatedElementsVector();
+}
+
const Vector<HTMLImageElement*>& HTMLFormControlsCollection::formImageElements() const
{
ASSERT(is<HTMLFormElement>(ownerNode()));
@@ -88,7 +97,8 @@
HTMLElement* HTMLFormControlsCollection::customElementAfter(Element* current) const
{
- const Vector<FormAssociatedElement*>& elements = formControlElements();
+ ScriptDisallowedScope::InMainThread scriptDisallowedScope;
+ auto& elements = unsafeFormControlElements();
unsigned start;
if (!current)
start = 0;
@@ -118,7 +128,8 @@
bool ownerIsFormElement = is<HTMLFormElement>(ownerNode());
HashSet<AtomicStringImpl*> foundInputElements;
- for (auto& elementPtr : formControlElements()) {
+ ScriptDisallowedScope::InMainThread scriptDisallowedScope;
+ for (auto& elementPtr : unsafeFormControlElements()) {
FormAssociatedElement& associatedElement = *elementPtr;
if (associatedElement.isEnumeratable()) {
HTMLElement& element = associatedElement.asHTMLElement();
Modified: trunk/Source/WebCore/html/HTMLFormControlsCollection.h (227478 => 227479)
--- trunk/Source/WebCore/html/HTMLFormControlsCollection.h 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebCore/html/HTMLFormControlsCollection.h 2018-01-24 08:19:16 UTC (rev 227479)
@@ -51,7 +51,8 @@
void invalidateCacheForDocument(Document&) override;
void updateNamedElementCache() const override;
- const Vector<FormAssociatedElement*>& formControlElements() const;
+ const Vector<FormAssociatedElement*>& unsafeFormControlElements() const;
+ Vector<Ref<FormAssociatedElement>> copyFormControlElementsVector() const;
const Vector<HTMLImageElement*>& formImageElements() const;
mutable Element* m_cachedElement;
Modified: trunk/Source/WebCore/html/HTMLFormElement.cpp (227478 => 227479)
--- trunk/Source/WebCore/html/HTMLFormElement.cpp 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebCore/html/HTMLFormElement.cpp 2018-01-24 08:19:16 UTC (rev 227479)
@@ -47,6 +47,7 @@
#include "Page.h"
#include "RadioNodeList.h"
#include "RenderTextControl.h"
+#include "ScriptDisallowedScope.h"
#include "Settings.h"
#include "UserGestureIndicator.h"
#include <limits>
@@ -851,6 +852,19 @@
document().formController().restoreControlStateIn(*this);
}
+const Vector<FormAssociatedElement*>& HTMLFormElement::unsafeAssociatedElements() const
+{
+ ASSERT(!ScriptDisallowedScope::InMainThread::isScriptAllowed());
+ return m_associatedElements;
+}
+
+Vector<Ref<FormAssociatedElement>> HTMLFormElement::copyAssociatedElementsVector() const
+{
+ return WTF::map(m_associatedElements, [] (auto* rawElement) {
+ return Ref<FormAssociatedElement>(*rawElement);
+ });
+}
+
void HTMLFormElement::copyNonAttributePropertiesFromElement(const Element& source)
{
m_wasDemoted = static_cast<const HTMLFormElement&>(source).m_wasDemoted;
Modified: trunk/Source/WebCore/html/HTMLFormElement.h (227478 => 227479)
--- trunk/Source/WebCore/html/HTMLFormElement.h 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebCore/html/HTMLFormElement.h 2018-01-24 08:19:16 UTC (rev 227479)
@@ -116,7 +116,8 @@
RadioButtonGroups& radioButtonGroups() { return m_radioButtonGroups; }
- const Vector<FormAssociatedElement*>& associatedElements() const { return m_associatedElements; }
+ WEBCORE_EXPORT const Vector<FormAssociatedElement*>& unsafeAssociatedElements() const;
+ Vector<Ref<FormAssociatedElement>> copyAssociatedElementsVector() const;
const Vector<HTMLImageElement*>& imageElements() const { return m_imageElements; }
StringPairVector textFieldValues() const;
Modified: trunk/Source/WebCore/loader/FormSubmission.cpp (227478 => 227479)
--- trunk/Source/WebCore/loader/FormSubmission.cpp 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebCore/loader/FormSubmission.cpp 2018-01-24 08:19:16 UTC (rev 227479)
@@ -182,11 +182,7 @@
StringPairVector formValues;
bool containsPasswordData = false;
- auto protectedAssociatedElements = WTF::map(form.associatedElements(), [] (auto* rawElement) {
- return Ref<FormAssociatedElement>(*rawElement);
- });
-
- for (auto& control : protectedAssociatedElements) {
+ for (auto& control : form.copyAssociatedElementsVector()) {
auto& element = control->asHTMLElement();
if (!element.isDisabledFormControl())
control->appendFormData(domFormData, isMultiPartForm);
Modified: trunk/Source/WebKitLegacy/mac/ChangeLog (227478 => 227479)
--- trunk/Source/WebKitLegacy/mac/ChangeLog 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebKitLegacy/mac/ChangeLog 2018-01-24 08:19:16 UTC (rev 227479)
@@ -1,3 +1,18 @@
+2018-01-23 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Harden against layout passes triggered when iterating through HTMLFormElement::associatedElements
+ https://bugs.webkit.org/show_bug.cgi?id=182037
+ <rdar://problem/36747812>
+
+ Reviewed by Ryosuke Niwa.
+
+ Rename associatedElements() to unsafeAssociatedElements(), and add ScriptDisallowedScopes. See WebCore ChangeLog
+ for more details.
+
+ * WebView/WebHTMLRepresentation.mm:
+ (-[WebHTMLRepresentation elementWithName:inForm:]):
+ (-[WebHTMLRepresentation controlsInForm:]):
+
2018-01-23 Alex Christensen <achristen...@webkit.org>
Use CompletionHandlers for ResourceHandleClient::didReceiveResponseAsync
Modified: trunk/Source/WebKitLegacy/mac/WebView/WebHTMLRepresentation.mm (227478 => 227479)
--- trunk/Source/WebKitLegacy/mac/WebView/WebHTMLRepresentation.mm 2018-01-24 07:04:06 UTC (rev 227478)
+++ trunk/Source/WebKitLegacy/mac/WebView/WebHTMLRepresentation.mm 2018-01-24 08:19:16 UTC (rev 227479)
@@ -58,6 +58,7 @@
#import <WebCore/NodeTraversal.h>
#import <WebCore/Range.h>
#import <WebCore/RenderElement.h>
+#import <WebCore/ScriptDisallowedScope.h>
#import <WebCore/TextResourceDecoder.h>
#import <WebKitLegacy/DOMHTMLInputElement.h>
#import <yarr/RegularExpression.h>
@@ -281,7 +282,9 @@
HTMLFormElement* formElement = formElementFromDOMElement(form);
if (!formElement)
return nil;
- const Vector<FormAssociatedElement*>& elements = formElement->associatedElements();
+
+ ScriptDisallowedScope::InMainThread scriptDisallowedScope;
+ auto& elements = formElement->unsafeAssociatedElements();
AtomicString targetName = name;
for (unsigned i = 0; i < elements.size(); i++) {
FormAssociatedElement& element = *elements[i];
@@ -329,7 +332,9 @@
if (!formElement)
return nil;
NSMutableArray *results = nil;
- const Vector<FormAssociatedElement*>& elements = formElement->associatedElements();
+
+ ScriptDisallowedScope::InMainThread scriptDisallowedScope;
+ auto& elements = formElement->unsafeAssociatedElements();
for (unsigned i = 0; i < elements.size(); i++) {
if (elements[i]->isEnumeratable()) { // Skip option elements, other duds
DOMElement *element = kit(&elements[i]->asHTMLElement());
