1992113 - Crash [@ mozilla::gfx::RecordedFilterNodeSetAttribute::PlayEvent] with out-of-bounds read

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Christian Holler (:decoder)]

Found during experimental IPC fuzzing on canvas translation:

==1935==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fffd1bb5fd5 bp 0x7fff6ee5c9d0 sp 0x7fff6ee5c8e0 T44)                                                                                                                                                                                      
==1935==The signal is caused by a READ memory access.                                                                                                       
==1935==Hint: address points to the zero page.                                                                                                              
    #0 0x7fffd1bb5fd5 in mozilla::gfx::RecordedFilterNodeSetAttribute::PlayEvent(mozilla::gfx::Translator*) const gfx/2d/RecordedEventImpl.h:4539:5
    #1 0x7fffd237c73d in mozilla::layers::CanvasTranslator::TranslateRecording()::$_1::operator()(mozilla::gfx::RecordedEvent*) const gfx/layers/ipc/CanvasTranslator.cpp:828:33
    #2 0x7fffd237c73d in bool std::__invoke_impl<bool, mozilla::layers::CanvasTranslator::TranslateRecording()::$_1&, mozilla::gfx::RecordedEvent*>(std::__invoke_other, mozilla::layers::CanvasTranslator::TranslateRecording()::$_1&, mozilla::gfx::RecordedEvent*&&) sysroot-x86_64-linux-gnu/
usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:60:14                                                                                                                                                                                                                                           
    #3 0x7fffd237c73d in std::enable_if<is_invocable_r_v<bool, mozilla::layers::CanvasTranslator::TranslateRecording()::$_1&, mozilla::gfx::RecordedEvent*>, bool>::type std::__invoke_r<bool, mozilla::layers::CanvasTranslator::TranslateRecording()::$_1&, mozilla::gfx::RecordedEvent*>(mozilla::layers::CanvasTransl
ator::TranslateRecording()::$_1&, mozilla::gfx::RecordedEvent*&&) sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:113:9                                                                  
    #4 0x7fffd237c73d in std::_Function_handler<bool (mozilla::gfx::RecordedEvent*), mozilla::layers::CanvasTranslator::TranslateRecording()::$_1>::_M_invoke(std::_Any_data const&, mozilla::gfx::RecordedEvent*&&) sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/std_function.h:291:9                                                                                                                           
    #5 0x7fffd1bf75ec in std::function<bool (mozilla::gfx::RecordedEvent*)>::operator()(mozilla::gfx::RecordedEvent*) const sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/std_function.h:622:14                                                           #6 0x7fffd1bb9213 in bool mozilla::gfx::RecordedEvent::DoWithEvent<mozilla::gfx::MemReader>(mozilla::gfx::MemReader&, mozilla::gfx::RecordedEvent::EventType, std::function<bool (mozilla::gfx::RecordedEvent*)> const&) gfx/2d/RecordedEventImpl.h:4836:5
    #7 0x7fffd22f2a9f in mozilla::layers::CanvasTranslator::TranslateRecording() gfx/layers/ipc/CanvasTranslator.cpp:800:20                                                                                                                                                                   
    #8 0x7fffd22f0c9b in mozilla::layers::CanvasTranslator::RecvInitTranslator(mozilla::layers::TextureType, mozilla::layers::TextureType, mozilla::gfx::BackendType, mozilla::ipc::shared_memory::Handle<(mozilla::ipc::shared_memory::Type)0>&&, nsTArray<mozilla::ipc::shared_memory::Handle<(mozilla::ipc::shared_mem
ory::Type)1> >&&, mozilla::ipc::shared_memory::Handle<(mozilla::ipc::shared_memory::Type)0>&&, mozilla::ipc::shared_memory::Handle<(mozilla::ipc::shared_memory::Type)0>&&) gfx/layers/ipc/CanvasTranslator.cpp:228:31
    #9 0x7fffd202699a in mozilla::layers::PCanvasParent::OnMessageReceived(IPC::Message const&) objdir/ipc/ipdl/PCanvasParent.cpp:447:84                          
    #10 0x7fffd30fca0b in mozilla::gfx::PCanvasManagerParent::OnMessageReceived(IPC::Message const&) objdir/ipc/ipdl/PCanvasManagerParent.cpp:261:32                  
    #11 0x7fffd0ce0136 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) ipc/glue/MessageChannel.cpp:1797:25
    #12 0x7fffd0cdbe25 in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, std::unique_ptr<IPC::Message, std::default_delete<IPC::Message> >) ipc/glue/MessageChannel.cpp:1723:9
    #13 0x7fffd0cdcebe in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) ipc/glue/MessageChannel.cpp:1512:3
    #14 0x7fffd0cde67e in mozilla::ipc::MessageChannel::MessageTask::Run() ipc/glue/MessageChannel.cpp:1614:14                                                                                                                                                                                
    #15 0x7fffcee4db81 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1151:16                         
    #16 0x7fffcee5b0c3 in NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:462:10                                                                                                                                                                                        
    #17 0x7fffd0ceacfa in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:299:20                                                                 
    #18 0x7fffd0aea0bd in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:373:10                                                                                                                                                                                             
    #19 0x7fffd0aea0bd in MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:366:3             
    #20 0x7fffd0aea0bd in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:348:3
    #21 0x7fffcee44e36 in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:366:10                                                                                                                                                                                                       
    #22 0x7ffff7f7a7a6 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:191:3                    
    #23 0x5555557048a6 in asan_thread_start(void*) _asan_rtl_:28                                                                                            
    #24 0x7fffb8c9caa3 in pthread_condattr_setpshared ??:?                                                                                                  
    #25 0x7fffb8d29c3b in __clone ??:?                                                                                                                      
                                                                                                                                                            
==1935==Register values:                                                                                                                                    
rax = 0x00007fffef19c1c0  rbx = 0x00007fff6ee5c8e0  rcx = 0x00000ffffde33838  rdx = 0x00007fff6d7745f0
rdi = 0x0000000000000000  rsi = 0x0000000000000000  rbp = 0x00007fff6ee5c9d0  rsp = 0x00007fff6ee5c8e0
 r8 = 0x000051a000870d53   r9 = 0x0000000000000701  r10 = 0x0000100078b13ba8  r11 = 0x000010007efef228
r12 = 0x000050b0003108a0  r13 = 0x00007fff6d7745f0  r14 = 0x00007fff6d41cc00  r15 = 0x000010006da7b980

At first this looks like a null deref from the trace, but it is actually an out-of-bounds read on an STL vector. What's happening is that here:

https://searchfox.org/firefox-main/rev/6f3037f71baa242161ed343cdccb12c1b8e50227/gfx/2d/RecordedEventImpl.h#4523-4530

We can read a size of 0 and don't abort. Hence, the resulting mPayload vector is empty.

When we later play the event, we call front on this vector:

https://searchfox.org/firefox-main/rev/6f3037f71baa242161ed343cdccb12c1b8e50227/gfx/2d/RecordedEventImpl.h#4480

which likely happens to return a null, but it's actually reading out of bounds.

I believe Lee is in the process of rewriting this part of the code, we should make sure though that the flaw isn't present anymore in the refactored code (and filing this also for documentation purposes, in case it gets externally reported as well).

Comment 1

[Christian Holler (:decoder)]

Looking further at this code, it is also not clear to me how validation happens that the payload is large enough to read the specified type.

Comment 2

[Lee Salzman [:lsalzman]]

This is fixed in my changes in bug 1991899. We will just need to make sure we uplift those fixes.

Comment 3

[Lee Salzman [:lsalzman]]

This is fixed by bug 1991899 now.

ESR 115 and release might need a determination. ESR 115 would require its own patch.

Comment 4

[Lee Salzman [:lsalzman]]

Attachment: (secure)

Comment 5

[Ryan VanderMeulen (PTO, back July 27)]

Are you saying that for ESR115, this patch is needed in addition to the other ones from bug 1991899 or is just this one sufficient?

Comment 6

[Lee Salzman [:lsalzman]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)

Are you saying that for ESR115, this patch is needed in addition to the other ones from bug 1991899 or is just this one sufficient?

Just this patch is sufficient for ESR 115, as the ones from bug 1991899 don't apply cleanly.

Comment 7

[Lee Salzman [:lsalzman]]

Comment on attachment 9517954 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Unknown
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Unknown
• Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: All. Fixes have already been landed for nightly, beta, and ESR 140 as part of a refactoring that took place in bug 1991899, but ESR 115 is still affected.
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 8

[Ryan VanderMeulen (PTO, back July 27)]

Comment on attachment 9517954 [details]
(secure)

Doesn't need sec-approval

Comment 9

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/0f14e81e4d27