Closed
Bug 1388243
Opened 7 years ago
Closed 7 years ago
Heap-use-after-free in mozilla::MediaStreamGraphImpl::UpdateMainThreadState
Categories
(Core :: Audio/Video, defect, P1)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | + | fixed |
firefox57 | + | fixed |
People
(Reporter: inferno, Assigned: padenot)
Details
(Keywords: csectype-uaf, sec-high)
Attachments
(2 files)
120 bytes,
text/html
|
Details | |
1.16 KB,
patch
|
jesup
:
review+
gchang
:
approval-mozilla-beta+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
================================================================= ==66387==ERROR: AddressSanitizer: heap-use-after-free on address 0x60b0000d8758 at pc 0x7f787015500b bp 0x7f783a3fdbd0 sp 0x7f783a3fdbc8 READ of size 4 at 0x60b0000d8758 thread T103 (MediaStreamGrph) #0 0x7f787015500a in mozilla::SystemClockDriver::WaitForNextIteration() /build/firefox/src/dom/media/GraphDriver.cpp:420:7 #1 0x7f78703e964b in mozilla::MediaStreamGraphImpl::UpdateMainThreadState() /build/firefox/src/dom/media/MediaStreamGraph.cpp:1439:20 #2 0x7f78703e99d0 in mozilla::MediaStreamGraphImpl::OneIteration(long) /build/firefox/src/dom/media/MediaStreamGraph.cpp:1471:10 #3 0x7f787015385b in mozilla::ThreadedDriver::RunThread() /build/firefox/src/dom/media/GraphDriver.cpp:340:35 #4 0x7f78701817cf in mozilla::MediaStreamGraphInitThreadRunnable::Run() /build/firefox/src/dom/media/GraphDriver.cpp:209:14 #5 0x7f7869ce88b6 in nsThread::ProcessNextEvent(bool, bool*) /build/firefox/src/xpcom/threads/nsThread.cpp:1446:14 #6 0x7f7869cf05a6 in NS_ProcessNextEvent(nsIThread*, bool) /build/firefox/src/xpcom/threads/nsThreadUtils.cpp:480:10 #7 0x7f786ae57e08 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /build/firefox/src/ipc/glue/MessagePump.cpp:339:20 #8 0x7f786ad93814 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:326:10 #9 0x7f786ad93814 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:319 #10 0x7f786ad93814 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:299 #11 0x7f7869cde85c in nsThread::ThreadFunc(void*) /build/firefox/src/xpcom/threads/nsThread.cpp:506:11 #12 0x7f78866d0d0a in _pt_root /build/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:216:5 #13 0x7f788a029183 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8183) #14 0x7f7889129ffc in clone (/lib/x86_64-linux-gnu/libc.so.6+0xfdffc) 0x60b0000d8758 is located 40 bytes inside of 104-byte region [0x60b0000d8730,0x60b0000d8798) freed by thread T106 (MediaStreamGrph) here: #0 0x4e9550 in __interceptor_free /build/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:47 #1 0x7f7870181d5d in Release /build/firefox/src/dom/media/GraphDriver.h:112:3 #2 0x7f7870181d5d in Release /build/firefox/src/objdir-ff-asan/dist/include/mozilla/RefPtr.h:40 #3 0x7f7870181d5d in Release /build/firefox/src/objdir-ff-asan/dist/include/mozilla/RefPtr.h:395 #4 0x7f7870181d5d in ~RefPtr /build/firefox/src/objdir-ff-asan/dist/include/mozilla/RefPtr.h:78 #5 0x7f7870181d5d in ~MediaStreamGraphInitThreadRunnable /build/firefox/src/dom/media/GraphDriver.cpp:172 #6 0x7f7870181d5d in mozilla::MediaStreamGraphInitThreadRunnable::~MediaStreamGraphInitThreadRunnable() /build/firefox/src/dom/media/GraphDriver.cpp:172 #7 0x7f7869cf778e in mozilla::Runnable::Release() /build/firefox/src/xpcom/threads/nsThreadUtils.cpp:44:1 #8 0x7f7869ce8a8c in ~nsCOMPtr_base /build/firefox/src/objdir-ff-asan/dist/include/nsCOMPtr.h:294:7 #9 0x7f7869ce8a8c in nsThread::ProcessNextEvent(bool, bool*) /build/firefox/src/xpcom/threads/nsThread.cpp:1452 #10 0x7f7869cf05a6 in NS_ProcessNextEvent(nsIThread*, bool) /build/firefox/src/xpcom/threads/nsThreadUtils.cpp:480:10 #11 0x7f786ae57e08 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /build/firefox/src/ipc/glue/MessagePump.cpp:339:20 #12 0x7f786ad93814 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:326:10 #13 0x7f786ad93814 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:319 #14 0x7f786ad93814 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:299 #15 0x7f7869cde85c in nsThread::ThreadFunc(void*) /build/firefox/src/xpcom/threads/nsThread.cpp:506:11 #16 0x7f78866d0d0a in _pt_root /build/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:216:5 #17 0x7f788a029183 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8183) previously allocated by thread T102 (CubebOp~tion #1) here: #0 0x4e98b8 in malloc /build/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:67 #1 0x524b7d in moz_xmalloc /build/firefox/src/memory/mozalloc/mozalloc.cpp:83:17 #2 0x7f78701576ce in operator new /build/firefox/src/objdir-ff-asan/dist/include/mozilla/mozalloc.h:200:12 #3 0x7f78701576ce in mozilla::AudioCallbackDriver::Init() /build/firefox/src/dom/media/GraphDriver.cpp:732 #4 0x7f7870156577 in mozilla::AsyncCubebTask::Run() /build/firefox/src/dom/media/GraphDriver.cpp:526:21 #5 0x7f7869cf4c01 in nsThreadPool::Run() /build/firefox/src/xpcom/threads/nsThreadPool.cpp:225:14 #6 0x7f7869cf53c0 in non-virtual thunk to nsThreadPool::Run() /build/firefox/src/xpcom/threads/nsThreadPool.cpp #7 0x7f7869ce88b6 in nsThread::ProcessNextEvent(bool, bool*) /build/firefox/src/xpcom/threads/nsThread.cpp:1446:14 #8 0x7f7869cf05a6 in NS_ProcessNextEvent(nsIThread*, bool) /build/firefox/src/xpcom/threads/nsThreadUtils.cpp:480:10 #9 0x7f786ae57e08 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /build/firefox/src/ipc/glue/MessagePump.cpp:339:20 #10 0x7f786ad93814 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:326:10 #11 0x7f786ad93814 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:319 #12 0x7f786ad93814 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:299 #13 0x7f7869cde85c in nsThread::ThreadFunc(void*) /build/firefox/src/xpcom/threads/nsThread.cpp:506:11 #14 0x7f78866d0d0a in _pt_root /build/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:216:5 #15 0x7f788a029183 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8183) Thread T103 (MediaStreamGrph) created by T101 (threaded-ml) here: #0 0x43656d in __interceptor_pthread_create /build/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:313 #1 0x7f78866cce0d in _PR_CreateThread /build/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:457:14 #2 0x7f78866cc91a in PR_CreateThread /build/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:548:12 #3 0x7f7869ce1676 in nsThread::Init(nsACString const&) /build/firefox/src/xpcom/threads/nsThread.cpp:688:8 #4 0x7f7869cef391 in nsThreadManager::NewNamedThread(nsACString const&, unsigned int, nsIThread**) /build/firefox/src/xpcom/threads/nsThreadManager.cpp:274:22 #5 0x7f7869cf354c in NS_NewNamedThread(nsACString const&, nsIThread**, nsIRunnable*, unsigned int) /build/firefox/src/xpcom/threads/nsThreadUtils.cpp:104:45 #6 0x7f78701520ae in NS_NewNamedThread<16> /build/firefox/src/objdir-ff-asan/dist/include/nsThreadUtils.h:72:10 #7 0x7f78701520ae in mozilla::ThreadedDriver::Start() /build/firefox/src/dom/media/GraphDriver.cpp:225 #8 0x7f787015febc in mozilla::AudioCallbackDriver::StateCallback(cubeb_state) /build/firefox/src/dom/media/GraphDriver.cpp:1078:17 #9 0x7f78793ca708 in cubeb_pulse::backend::stream::{{impl}}::state_change_callback /build/firefox/src/media/libcubeb/cubeb-pulse-rs/src/backend/stream.rs:651 #10 0x7f78793ca708 in cubeb_pulse::backend::stream::{{impl}}::new::check_error /build/firefox/src/media/libcubeb/cubeb-pulse-rs/src/backend/stream.rs:107 #11 0x7f78793ca708 in core::ops::Fn::call<fn(&pulse::stream::Stream, *mut std::os::raw::c_void),(&pulse::stream::Stream, *mut std::os::raw::c_void)> /checkout/src/libcore/ops.rs:2588 #12 0x7f78793ca708 in pulse::stream::Stream::set_state_callback::wrapped::h9463fe3761c37711 /build/firefox/src/media/libcubeb/cubeb-pulse-rs/pulse-rs/src/stream.rs:230 #13 0x7f78411cfc91 (/usr/lib/x86_64-linux-gnu/libpulse.so.0+0x28c91) Thread T101 (threaded-ml) created by T0 (Web Content) here: #0 0x43656d in __interceptor_pthread_create /build/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:313 #1 0x7f7840d7a74c in pa_thread_new (/usr/lib/x86_64-linux-gnu/pulseaudio/libpulsecommon-4.0.so+0x4574c) Thread T106 (MediaStreamGrph) created by T102 (CubebOp~tion #1) here: #0 0x43656d in __interceptor_pthread_create /build/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:313 #1 0x7f78866cce0d in _PR_CreateThread /build/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:457:14 #2 0x7f78866cc91a in PR_CreateThread /build/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:548:12 #3 0x7f7869ce1676 in nsThread::Init(nsACString const&) /build/firefox/src/xpcom/threads/nsThread.cpp:688:8 #4 0x7f7869cef391 in nsThreadManager::NewNamedThread(nsACString const&, unsigned int, nsIThread**) /build/firefox/src/xpcom/threads/nsThreadManager.cpp:274:22 #5 0x7f7869cf354c in NS_NewNamedThread(nsACString const&, nsIThread**, nsIRunnable*, unsigned int) /build/firefox/src/xpcom/threads/nsThreadUtils.cpp:104:45 #6 0x7f78701520ae in NS_NewNamedThread<16> /build/firefox/src/objdir-ff-asan/dist/include/nsThreadUtils.h:72:10 #7 0x7f78701520ae in mozilla::ThreadedDriver::Start() /build/firefox/src/dom/media/GraphDriver.cpp:225 #8 0x7f7870157ba2 in mozilla::AudioCallbackDriver::Init() /build/firefox/src/dom/media/GraphDriver.cpp:740:19 #9 0x7f7870156577 in mozilla::AsyncCubebTask::Run() /build/firefox/src/dom/media/GraphDriver.cpp:526:21 #10 0x7f7869cf4c01 in nsThreadPool::Run() /build/firefox/src/xpcom/threads/nsThreadPool.cpp:225:14 #11 0x7f7869cf53c0 in non-virtual thunk to nsThreadPool::Run() /build/firefox/src/xpcom/threads/nsThreadPool.cpp #12 0x7f7869ce88b6 in nsThread::ProcessNextEvent(bool, bool*) /build/firefox/src/xpcom/threads/nsThread.cpp:1446:14 #13 0x7f7869cf05a6 in NS_ProcessNextEvent(nsIThread*, bool) /build/firefox/src/xpcom/threads/nsThreadUtils.cpp:480:10 #14 0x7f786ae57e08 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /build/firefox/src/ipc/glue/MessagePump.cpp:339:20 #15 0x7f786ad93814 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:326:10 #16 0x7f786ad93814 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:319 #17 0x7f786ad93814 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:299 #18 0x7f7869cde85c in nsThread::ThreadFunc(void*) /build/firefox/src/xpcom/threads/nsThread.cpp:506:11 #19 0x7f78866d0d0a in _pt_root /build/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:216:5 #20 0x7f788a029183 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8183) Thread T102 (CubebOp~tion #1) created by T0 (Web Content) here: #0 0x43656d in __interceptor_pthread_create /build/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:313 #1 0x7f78866cce0d in _PR_CreateThread /build/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:457:14 #2 0x7f78866cc91a in PR_CreateThread /build/firefox/src/nsprpub/pr/src/pthreads/ptthread.c:548:12 #3 0x7f7869ce1676 in nsThread::Init(nsACString const&) /build/firefox/src/xpcom/threads/nsThread.cpp:688:8 #4 0x7f7869cef391 in nsThreadManager::NewNamedThread(nsACString const&, unsigned int, nsIThread**) /build/firefox/src/xpcom/threads/nsThreadManager.cpp:274:22 #5 0x7f7869cf2ca6 in NS_NewNamedThread /build/firefox/src/xpcom/threads/nsThreadUtils.cpp:104:45 #6 0x7f7869cf2ca6 in nsThreadPool::PutEvent(already_AddRefed<nsIRunnable>, unsigned int) /build/firefox/src/xpcom/threads/nsThreadPool.cpp:107 #7 0x7f7869cf5766 in nsThreadPool::Dispatch(already_AddRefed<nsIRunnable>, unsigned int) /build/firefox/src/xpcom/threads/nsThreadPool.cpp:274:5 #8 0x7f7869cbfd59 in mozilla::SharedThreadPool::Dispatch(already_AddRefed<nsIRunnable>, unsigned int) /build/firefox/src/objdir-ff-asan/dist/include/mozilla/SharedThreadPool.h:71:68 #9 0x7f787015b866 in Dispatch /build/firefox/src/objdir-ff-asan/dist/include/nsIEventTarget.h:37:14 #10 0x7f787015b866 in Dispatch /build/firefox/src/dom/media/GraphDriver.h:555 #11 0x7f787015b866 in mozilla::AudioCallbackDriver::Start() /build/firefox/src/dom/media/GraphDriver.cpp:802 #12 0x7f78703eb9e0 in mozilla::MediaStreamGraphImpl::RunInStableState(bool) /build/firefox/src/dom/media/MediaStreamGraph.cpp:1776:17 #13 0x7f787041a8bd in mozilla::(anonymous namespace)::MediaStreamGraphStableStateRunnable::Run() /build/firefox/src/dom/media/MediaStreamGraph.cpp:1631:15 #14 0x7f7869afcf06 in mozilla::CycleCollectedJSContext::ProcessStableStateQueue() /build/firefox/src/xpcom/base/CycleCollectedJSContext.cpp:309:12 #15 0x7f786ba5b329 in XPCJSContext::AfterProcessTask(unsigned int) /build/firefox/src/js/xpconnect/src/XPCJSContext.cpp:1010:30 #16 0x7f7869ce8f7c in nsThread::ProcessNextEvent(bool, bool*) /build/firefox/src/xpcom/threads/nsThread.cpp:1462:24 #17 0x7f7869cf05a6 in NS_ProcessNextEvent(nsIThread*, bool) /build/firefox/src/xpcom/threads/nsThreadUtils.cpp:480:10 #18 0x7f786ae565d9 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /build/firefox/src/ipc/glue/MessagePump.cpp:125:5 #19 0x7f786ad93814 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:326:10 #20 0x7f786ad93814 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:319 #21 0x7f786ad93814 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:299 #22 0x7f7871c6ef92 in nsBaseAppShell::Run() /build/firefox/src/widget/nsBaseAppShell.cpp:156:27 #23 0x7f7876dc9537 in XRE_RunAppShell() /build/firefox/src/toolkit/xre/nsEmbedFunctions.cpp:882:22 #24 0x7f786ad93814 in RunInternal /build/firefox/src/ipc/chromium/src/base/message_loop.cc:326:10 #25 0x7f786ad93814 in RunHandler /build/firefox/src/ipc/chromium/src/base/message_loop.cc:319 #26 0x7f786ad93814 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:299 #27 0x7f7876dc8eec in XRE_InitChildProcess(int, char**, XREChildData const*) /build/firefox/src/toolkit/xre/nsEmbedFunctions.cpp:699:34 #28 0x523945 in content_process_main /build/firefox/src/browser/app/../../ipc/contentproc/plugin-container.cpp:64:30 #29 0x523945 in main /build/firefox/src/browser/app/nsBrowserApp.cpp:285 #30 0x7f788904df44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44) SUMMARY: AddressSanitizer: heap-use-after-free /build/firefox/src/dom/media/GraphDriver.cpp:420:7 in mozilla::SystemClockDriver::WaitForNextIteration() Shadow bytes around the buggy address: 0x0c1680013090: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa 0x0c16800130a0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd 0x0c16800130b0: fd fa fa fa fa fa fa fa fa fa fd fd fd fd fd fd 0x0c16800130c0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa 0x0c16800130d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa =>0x0c16800130e0: fa fa fa fa fa fa fd fd fd fd fd[fd]fd fd fd fd 0x0c16800130f0: fd fd fd fa fa fa fa fa fa fa fa fa fd fd fd fd 0x0c1680013100: fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa 0x0c1680013110: fa fa fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c1680013120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1680013130: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 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 Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==66387==ABORTING
Updated•7 years ago
|
Group: core-security → media-core-security
Keywords: csectype-uaf,
sec-critical
Comment 1•7 years ago
|
||
Anthony, do you know who might be able to look at this? Thanks.
Flags: needinfo?(ajones)
Paul would likely be the right person but cc-ing karlt just in case.
Flags: needinfo?(ajones)
Assignee | ||
Comment 4•7 years ago
|
||
I can't repro, but it's likely that we're seeing the following scenario: An `AudioContext` is created. The main thread creates a `CubebOperation` thread [0], to open an audio stream using cubeb (we do not create audio streams from the main thread, because it's a blocking operation that can take a long time). This thread runs `AudioCallbackDriver::Init`. Here, the stream creation fails (this can happen for various reasons, no audio hardware, maximum number of streams on the OS mixing server reached, etc.) [1]. From within cubeb, the state callback is called. Because the stream is now in error, we immediately create, set a current driver and start a new `SystemClockDriver` (let's call it [A]) to keep the MSG rolling [2]. Now, we return to the `cubeb_stream_init`. We see that it fails so we're going in the `else {}` clause [4]. This also creates a fallback `SystemClockDriver` (we'll call it [B]), immediately starts it. [1] is not referenced by anything (we see that it has been deleted when the `MediaStreamGraphInitRunnable` is being freed), which should never happen (drivers are always freed by the next driver, or by a shutdown runnable), because it should have been given to the graph, but the graph is now running off [B]. [A] has had the time to run a graph iteration, and tries to go sleep on its monitor, but since the monitor is owned by [A], it's freed. At this point the crash occur. The fix here is simple: don't try to double-fallback. See the attached patch. [0]: http://searchfox.org/mozilla-central/source/dom/media/GraphDriver.cpp#785 [1]: http://searchfox.org/mozilla-central/source/dom/media/GraphDriver.cpp#690 [2]: http://searchfox.org/mozilla-central/source/dom/media/GraphDriver.cpp#1050 [4]: http://searchfox.org/mozilla-central/source/dom/media/GraphDriver.cpp#705
Flags: needinfo?(padenot)
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #8898306 -
Flags: review?(rjesup)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → padenot
Status: NEW → ASSIGNED
Assignee | ||
Updated•7 years ago
|
Rank: 15
Priority: -- → P1
Comment hidden (typo) |
Updated•7 years ago
|
Attachment #8898306 -
Flags: review?(rjesup) → review+
Comment 7•7 years ago
|
||
I can reproduce here, and so tested the patch and could no longer reproduce, thanks! This is the start of a regression range on m-c pushdates: 20170716085807 crash 20170707083450 different/startup crash 20170701154225 different/startup crash 20170620003011 good
Assignee | ||
Comment 8•7 years ago
|
||
Comment on attachment 8898306 [details] [diff] [review] fix [Security approval request comment] How easily could an exploit be constructed based on the patch? This bug only occurs when creating an audio stream fails. For example, this can happen in the following scenarios, when a page tries to create an AudioContext or do a webrtc call on a machine that: - has no audio hardware / drivers - reached some kind of limit of the number of audio stream (this happens on some older OSX versions) - has exhausted a system resource necessary to open an audio stream (for example, file descriptors) In addition, the timing needs to be right (it's highly concurrent, with threads being created and destroyed a lot). It's a UAF on a object that has virtual members, though. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? It's non trivial to understand the cause (see comment 4), and this bug is caused in a module and manifests itself in an other. I'd say it's quite hard to find. I've added a comment, but I can remove it. Which older supported branches are affected by this flaw? 56 (current beta), 57 If not all supported branches, which bug introduced the flaw? bug 1374494, where the cubeb stream state callback started to be called in case of error during `cubeb_stream_init`, unlike all other backends. The fix will have to happen there as well (I'll get a patch upstream), but this patch provides an extra layer of security, since it will protect all platforms. We expect to have some movement in cubeb in the near future (we're rewriting it incrementally in rust), I would believe similar mistake can happen. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? This patch applies cleanly to 56 and 57. How likely is this patch to cause regressions; how much testing does it need? The cause and fix are well understood, and Karl has verified that this fixes it for him.
Attachment #8898306 -
Flags: sec-approval?
Assignee | ||
Comment 9•7 years ago
|
||
Dan, you might want to know about this one. In `cubeb-pulse-rs`, this is caused by the fact that the `state_callback` is called on error during the stream init. If the context is not in good shape, the callback is called. The state callback should only be called when we have succeeded opening a stream. It was never explicitly state I think, but that what all backends do.
Flags: needinfo?(dglastonbury)
Comment 10•7 years ago
|
||
Comment on attachment 8898306 [details] [diff] [review] fix sec-approval+ for trunk. We'll want a beta patch nominated as well to land after it goes onto trunk.
Attachment #8898306 -
Flags: sec-approval? → sec-approval+
Updated•7 years ago
|
status-firefox55:
--- → unaffected
status-firefox56:
--- → affected
status-firefox-esr52:
--- → unaffected
tracking-firefox56:
--- → +
tracking-firefox57:
--- → +
Comment 11•7 years ago
|
||
(In reply to Paul Adenot (:padenot) from comment #9) > Dan, you might want to know about this one. In `cubeb-pulse-rs`, this is > caused by the fact that the `state_callback` is called on error during the > stream init. If the context is not in good shape, the callback is called. > The state callback should only be called when we have succeeded opening a > stream. It was never explicitly state I think, but that what all backends do. Thanks for the heads up, Paul.
Flags: needinfo?(dglastonbury)
Assignee | ||
Comment 12•7 years ago
|
||
This applies cleanly on beta, I'm going to nominate it now.
Assignee | ||
Comment 13•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e4bb3a203a46f97ba0b5bb681bb1fb15e4474e4
Assignee | ||
Comment 14•7 years ago
|
||
Comment on attachment 8898306 [details] [diff] [review] fix Approval Request Comment [Feature/Bug causing the regression]: bug 1374494 [User impact if declined]: crash [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: it's been landed, but karl has checked that it fixes it [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: there is a good understanding of the issue [String changes made/needed]: none
Attachment #8898306 -
Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/1e4bb3a203a4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 16•7 years ago
|
||
Comment on attachment 8898306 [details] [diff] [review] fix Fix a sec-critical. Beta56+.
Attachment #8898306 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•7 years ago
|
Group: media-core-security → core-security-release
Updated•7 years ago
|
Flags: sec-bounty?
Updated•7 years ago
|
Group: core-security-release
Updated•7 years ago
|
Flags: sec-bounty? → sec-bounty+
Keywords: sec-critical → sec-high
You need to log in
before you can comment on or make changes to this bug.
Description
•