Closed Bug 1727729 Opened 2 years ago Closed 1 year ago

Crash [@ std::_Hashtable<...>::_M_bucket_index] through [@ mozilla::HostWebGLContext::AutoResolveT::As] due to invalid IPC message

Categories

(Core :: Graphics: CanvasWebGL, defect)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
95 Branch
Tracking Status
firefox-esr78 --- wontfix
firefox-esr91 --- wontfix
firefox93 --- wontfix
firefox94 --- wontfix
firefox95 --- fixed

People

(Reporter: decoder, Assigned: decoder)

Details

(4 keywords, Whiteboard: [post-critsmash-triage][adv-main95-])

Crash Data

Attachments

(2 files)

In experimental IPC fuzzing, we found the following crash on mozilla-central revision 610ae1bbeff8+ (patched fuzzing build):

=================================================================
==1717==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000110 (pc 0x7fffe35c5c72 bp 0x7fffc7456550 sp 0x7fffc74564a0 T24)
==1717==The signal is caused by a READ memory access.
==1717==Hint: address points to the zero page.
    #0 0x7fffe35c5c72 in std::_Hashtable<...>::_M_bucket_index(unsigned long const&, unsigned long) const include/c++/7.5.0/bits/hashtable.h:631
    #1 0x7fffe35c5c72 in std::_Hashtable<...>::find(unsigned long const&) const include/c++/7.5.0/bits/hashtable.h:1440
    #2 0x7fffe35c5c72 in std::unordered_map<...>::find(unsigned long const&) const include/c++/7.5.0/bits/unordered_map.h:924
    #3 0x7fffe35c5c72 in decltype (&((({parm#1}.find)({parm#2}))->second)) mozilla::MaybeFind<...>(std::unordered_map<...> const&, unsigned long const&) dist/include/mozilla/dom/WebGLTypes.h:891
    #4 0x7fffe35c5c72 in mozilla::HostWebGLContext::AutoResolveT::As(mozilla::WebGLRenderbuffer*) const dom/canvas/HostWebGLContext.h:132
    #5 0x7fffe35c5c72 in mozilla::WebGLRenderbuffer* mozilla::HostWebGLContext::ById<mozilla::WebGLRenderbuffer>(unsigned long) const dom/canvas/HostWebGLContext.h:158
    #6 0x7fffe35c5c72 in mozilla::HostWebGLContext::GetRenderbufferParameter(unsigned long, unsigned int) const dom/canvas/HostWebGLContext.h:386
    #7 0x7fffe35c5c72 in mozilla::dom::WebGLParent::RecvGetRenderbufferParameter(unsigned long, unsigned int, mozilla::Maybe<double>*) dom/canvas/WebGLParent.cpp:262
    #8 0x7fffde3f7c48 in mozilla::dom::PWebGLParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) ipc/ipdl/PWebGLParent.cpp:1421
    #9 0x7fffdd30249b in mozilla::layers::PCompositorManagerParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) ipc/ipdl/PCompositorManagerParent.cpp:504
    #10 0x7fffdce026ee in mozilla::ipc::MessageChannel::DispatchSyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&, IPC::Message*&) ipc/glue/MessageChannel.cpp:2032
    #11 0x7fffdcdff232 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) ipc/glue/MessageChannel.cpp:1986
    #12 0x7fffdce013b9 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) ipc/glue/MessageChannel.cpp:1828
    #13 0x7fffdce01d80 in mozilla::ipc::MessageChannel::MessageTask::Run() ipc/glue/MessageChannel.cpp:1866
    #14 0x7fffdb640eba in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1142
    #15 0x7fffdb64d8d0 in NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:466
    #16 0x7fffdce0daaa in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:300
    #17 0x7fffdcc3d955 in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:331
    #18 0x7fffdcc3d955 in MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:324
    #19 0x7fffdcc3d955 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:306
    [...]

The issue here seems pretty simple, we can call any of the methods in WebGLParent without having called WebGLParent::RecvInitialize before. so mHost is a null pointer and we end up with a lot of different near-null derefs. I think the WebGLParent should check if it has been initialized and return an IPC error if that is not the case. I have a patch for this.

Not a sec issue (null-deref at best as far as I can tell), but keeping locked to not draw attention to our IPC fuzzing attempts.

Severity: -- → S2
Crash Signature: [@ std::_Hashtable<unsigned long, std::pair<unsigned long const, RefPtr<mozilla::WebGLRenderbuffer> >, std::allocator<std::pair<unsigned long const, RefPtr<mozilla::WebGLRenderbuffer> > >, std::__detail::_Select1st, std::equal_to<unsigned long>, std::hash → [@ mozilla::HostWebGLContext::AutoResolveT::As]
Assignee: nobody → choller
Status: NEW → ASSIGNED
Keywords: sec-other

If this is a common issue, we could support this pattern in IPDL. You could specify that some message is an initializer, and the generated code could automatically add this state and checks.

I think this needs sec-approval.

This crash was a null deref, so I marked it sec-other and it doesn't need sec-approval prior to landing. We've left it hidden because it talks about the IPC fuzzing that is underway.

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main95-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.