1585760 - AddressSanitizer: heap-use-after-free [@ mozilla::layers::PersistentBufferProviderShared::Create] with READ of size 8

Reporter

Description

•

4 years ago

Found while fuzzing mozilla-central rev e545376c3391. I don't currently have a working testcase but will update if one becomes available.

==128192==ERROR: AddressSanitizer: heap-use-after-free on address 0x617000048ef8 at pc 0x7f7000f35d53 bp 0x7ffd0622a390 sp 0x7ffd0622a388
READ of size 8 at 0x617000048ef8 thread T0 (file:// Content)
    #0 0x7f7000f35d52 in mozilla::layers::PersistentBufferProviderShared::Create(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::SurfaceFormat, mozilla::layers::KnowsCompositor*) /src/gfx/layers/PersistentBufferProvider.cpp:103:49
    #1 0x7f70012eb83b in mozilla::layers::WebRenderLayerManager::CreatePersistentBufferProvider(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat) /src/gfx/layers/wr/WebRenderLayerManager.cpp:746:7
    #2 0x7f7004a6384b in mozilla::dom::CanvasRenderingContext2D::TrySharedTarget(RefPtr<mozilla::gfx::DrawTarget>&, RefPtr<mozilla::layers::PersistentBufferProvider>&) /src/dom/canvas/CanvasRenderingContext2D.cpp:1435:32
    #3 0x7f7004a6204b in mozilla::dom::CanvasRenderingContext2D::EnsureTarget(mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const*, bool) /src/dom/canvas/CanvasRenderingContext2D.cpp:1300:8
    #4 0x7f7004a7f76c in mozilla::dom::CanvasRenderingContext2D::EnsureWritablePath() /src/dom/canvas/CanvasRenderingContext2D.cpp:3068:3
    #5 0x7f7004a81650 in mozilla::dom::CanvasRenderingContext2D::Rect(double, double, double, double) /src/dom/canvas/CanvasRenderingContext2D.cpp:3030:3
    #6 0x7f700355ac48 in mozilla::dom::CanvasRenderingContext2D_Binding::rect(JSContext*, JS::Handle<JSObject*>, mozilla::dom::CanvasRenderingContext2D*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/CanvasRenderingContext2DBinding.cpp:5529:24
    #7 0x7f7004966516 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3250:13
    #8 0x7f700b5cd51c in CallJSNative /src/js/src/vm/Interpreter.cpp:458:13
    #9 0x7f700b5cd51c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /src/js/src/vm/Interpreter.cpp:551
    #10 0x7f700b5b5ba0 in CallFromStack /src/js/src/vm/Interpreter.cpp:624:10
    #11 0x7f700b5b5ba0 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3113
    #12 0x7f700b59733f in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:424:10
    #13 0x7f700b5ce026 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /src/js/src/vm/Interpreter.cpp:592:13
    #14 0x7f700b5d0379 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /src/js/src/vm/Interpreter.cpp:637:8
    #15 0x7f700c18b96b in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2728:10
    #16 0x7f700412aed0 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventListenerBinding.cpp:52:8
    #17 0x7f7005104765 in HandleEvent<mozilla::dom::EventTarget *> /src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:66:12
    #18 0x7f7005104765 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1033
    #19 0x7f70051061db in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) /src/dom/events/EventListenerManager.cpp:1231:17
    #20 0x7f70050ecbea in HandleEvent /src/obj-firefox/dist/include/mozilla/EventListenerManager.h:353:5
    #21 0x7f70050ecbea in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:349
    #22 0x7f70050eb402 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:551:16
    #23 0x7f70050f0dee in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:1045:11
    #24 0x7f7007ac2414 in nsDocumentViewer::LoadComplete(nsresult) /src/layout/base/nsDocumentViewer.cpp:1170:7
    #25 0x7f700a719ba1 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /src/docshell/base/nsDocShell.cpp:6568:20
    #26 0x7f700a718e1e in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:6346:7
    #27 0x7f700a71da5f in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp
    #28 0x7f70008fbd6c in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /src/uriloader/base/nsDocLoader.cpp:1354:3
    #29 0x7f70008fa9ec in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp:913:14
    #30 0x7f70008f65b9 in nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:732:9
    #31 0x7f70008f9466 in nsDocLoader::OnStopRequest(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp:620:5
    #32 0x7f70008fa5cc in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp
    #33 0x7f6ffe0a6730 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /src/netwerk/base/nsLoadGroup.cpp:568:22
    #34 0x7f7001fbbbf8 in DoUnblockOnload /src/dom/base/Document.cpp:10769:18
    #35 0x7f7001fbbbf8 in mozilla::dom::Document::UnblockOnload(bool) /src/dom/base/Document.cpp:10701
    #36 0x7f7005082fda in ~LoadBlockingAsyncEventDispatcher /src/dom/events/AsyncEventDispatcher.cpp:117:18
    #37 0x7f7005082fda in mozilla::LoadBlockingAsyncEventDispatcher::~LoadBlockingAsyncEventDispatcher() /src/dom/events/AsyncEventDispatcher.cpp:115
    #38 0x7f6ffde2357c in Release /src/xpcom/threads/nsThreadUtils.cpp:54:1
    #39 0x7f6ffde2357c in mozilla::CancelableRunnable::Release() /src/xpcom/threads/nsThreadUtils.cpp:76
    #40 0x7f6ffdde3c84 in assign_assuming_AddRef /src/obj-firefox/dist/include/nsCOMPtr.h:367:7
    #41 0x7f6ffdde3c84 in assign_assuming_AddRef /src/obj-firefox/dist/include/nsCOMPtr.h:390
    #42 0x7f6ffdde3c84 in operator= /src/obj-firefox/dist/include/nsCOMPtr.h:678
    #43 0x7f6ffdde3c84 in mozilla::SchedulerGroup::Runnable::Run() /src/xpcom/threads/SchedulerGroup.cpp:299
    #44 0x7f6ffde153b9 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1225:14
    #45 0x7f6ffde1c028 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #46 0x7f6fff0546ff in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #47 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #48 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #49 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #50 0x7f70073cb719 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #51 0x7f700b315c6f in XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:934:20
    #52 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #53 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #54 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #55 0x7f700b315516 in XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:769:34
    #56 0x5607b18b6bfa in content_process_main /src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #57 0x5607b18b6bfa in main /src/browser/app/nsBrowserApp.cpp:272
    #58 0x7f7020deeb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #59 0x5607b17d821c in _start (/home/worker/builds/m-c-20190930162137-fuzzing-asan-opt/firefox+0x4921c)

0x617000048ef8 is located 248 bytes inside of 648-byte region [0x617000048e00,0x617000049088)
freed by thread T0 (file:// Content) here:
    #0 0x5607b18838b2 in __interceptor_free /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x7f70016d2f7c in Release /src/obj-firefox/dist/include/mozilla/layers/CompositorBridgeChild.h:60:3
    #2 0x7f70016d2f7c in Release /src/gfx/layers/../../mfbt/RefPtr.h:48
    #3 0x7f70016d2f7c in Release /src/gfx/layers/../../mfbt/RefPtr.h:373
    #4 0x7f70016d2f7c in assign_assuming_AddRef /src/gfx/layers/../../mfbt/RefPtr.h:67
    #5 0x7f70016d2f7c in operator= /src/gfx/layers/../../mfbt/RefPtr.h:162
    #6 0x7f70016d2f7c in Revoke /src/obj-firefox/dist/include/nsThreadUtils.h:749
    #7 0x7f70016d2f7c in Revoke /src/obj-firefox/dist/include/nsThreadUtils.h:1190
    #8 0x7f70016d2f7c in ~RunnableMethodImpl /src/obj-firefox/dist/include/nsThreadUtils.h:1151
    #9 0x7f70016d2f7c in mozilla::detail::RunnableMethodImpl<RefPtr<mozilla::layers::CompositorBridgeChild>, void (mozilla::layers::CompositorBridgeChild::*)(), true, (mozilla::RunnableKind)0>::~RunnableMethodImpl() /src/obj-firefox/dist/include/nsThreadUtils.h:1151
    #10 0x7f6ffddfff4c in mozilla::Runnable::Release() /src/xpcom/threads/nsThreadUtils.cpp:54:1
    #11 0x7f6ffde15b9a in ~nsCOMPtr_base /src/obj-firefox/dist/include/nsCOMPtr.h:331:7
    #12 0x7f6ffde15b9a in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1272
    #13 0x7f6ffde1c028 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #14 0x7f6fff0546ff in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #15 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #16 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #17 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #18 0x7f70073cb719 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #19 0x7f700b315c6f in XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:934:20
    #20 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #21 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #22 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #23 0x7f700b315516 in XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:769:34
    #24 0x5607b18b6bfa in content_process_main /src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #25 0x5607b18b6bfa in main /src/browser/app/nsBrowserApp.cpp:272
    #26 0x7f7020deeb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

previously allocated by thread T0 (file:// Content) here:
    #0 0x5607b1883c33 in __interceptor_malloc /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
    #1 0x5607b18b92bd in moz_xmalloc /src/memory/mozalloc/mozalloc.cpp:52:15
    #2 0x7f70016973ab in operator new /src/obj-firefox/dist/include/mozilla/cxxalloc.h:33:10
    #3 0x7f70016973ab in mozilla::layers::CompositorManagerChild::CreateContentCompositorBridge(unsigned int) /src/gfx/layers/ipc/CompositorManagerChild.cpp:105
    #4 0x7f7006949501 in mozilla::dom::ContentChild::RecvInitRendering(mozilla::ipc::Endpoint<mozilla::layers::PCompositorManagerChild>&&, mozilla::ipc::Endpoint<mozilla::layers::PImageBridgeChild>&&, mozilla::ipc::Endpoint<mozilla::gfx::PVRManagerChild>&&, mozilla::ipc::Endpoint<mozilla::PRemoteDecoderManagerChild>&&, nsTArray<unsigned int>&&) /src/dom/ipc/ContentChild.cpp:1544:8
    #5 0x7f6fff3cb3b5 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PContentChild.cpp:8247:56
    #6 0x7f6fff04b316 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /src/ipc/glue/MessageChannel.cpp:2185:25
    #7 0x7f6fff045f6d in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /src/ipc/glue/MessageChannel.cpp:2109:9
    #8 0x7f6fff048597 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /src/ipc/glue/MessageChannel.cpp:1954:3
    #9 0x7f6fff049427 in mozilla::ipc::MessageChannel::MessageTask::Run() /src/ipc/glue/MessageChannel.cpp:1985:13
    #10 0x7f6ffde153b9 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1225:14
    #11 0x7f6ffde1c028 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #12 0x7f6fff0546ff in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #13 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #14 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #15 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #16 0x7f70073cb719 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #17 0x7f700b315c6f in XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:934:20
    #18 0x7f6ffef4d7b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #19 0x7f6ffef4d7b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #20 0x7f6ffef4d7b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #21 0x7f700b315516 in XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:769:34
    #22 0x5607b18b6bfa in content_process_main /src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #23 0x5607b18b6bfa in main /src/browser/app/nsBrowserApp.cpp:272
    #24 0x7f7020deeb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

SUMMARY: AddressSanitizer: heap-use-after-free /src/gfx/layers/PersistentBufferProvider.cpp:103:49 in mozilla::layers::PersistentBufferProviderShared::Create(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::SurfaceFormat, mozilla::layers::KnowsCompositor*)
Shadow bytes around the buggy address:
  0x0c2e80001180: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e80001190: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e800011a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e800011b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c2e800011c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c2e800011d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]
  0x0c2e800011e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e800011f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e80001200: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2e80001210: fd fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c2e80001220: 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
  Shadow gap:              cc
==128192==ABORTING

Tyson Smith [:tsmith]

Updated

•

4 years ago

Group: core-security → gfx-core-security

Jessie [:jbonisteel] pls NI

Updated

•

4 years ago

Priority: -- → P3

Tyson Smith [:tsmith]

Updated

•

4 years ago

Keywords: csectype-uaf, sec-high

Jessie [:jbonisteel] pls NI

Comment 1

•

4 years ago

Nical, any suggestions as to what might be causing this?

Flags: needinfo?(nical.bugzilla)

Nicolas Silva [:nical]

Assignee

Comment 2

•

4 years ago

It looks like we are shutting down or for some othre reason the CompositorBridgeChild is already shut down through the process of getting it as a TextureForwarder we use the IPDL-generated Manager() method to go from WebRenderBridgeChild to CompositorBirdgeChild and that doesn't check reference counting ot other mechanism.

Checking IPCOpen() before calling Manager() in WebRenderBridgeChild::GetCompositorBridgeChild should fix this.

Assignee: nobody → nical.bugzilla

Flags: needinfo?(nical.bugzilla)

Nicolas Silva [:nical]

Assignee

Comment 3

•

4 years ago

Attached file Bug 1585760 - Check that IPC is open before accessing parent protocol. r=sotaro — Details

Nicolas Silva [:nical]

Assignee

Comment 4

•

4 years ago

This is more complicated than anticipated. Between the time IPCOpen() starts returning false and when the parent protocol dies, there appears to be a number of uses of GetCompositorBridgeChild and similar methods with some mutex interactions leading to timeouts.

Release mgmt bot [:suhaib / :marco/ :calixte]

Comment 5

•

3 years ago

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:nical, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(nical.bugzilla)

Nicolas Silva [:nical]

Assignee

Updated

•

3 years ago

Flags: needinfo?(nical.bugzilla)

Nicolas Silva [:nical]

Assignee

Comment 6

•

3 years ago

Attached file Bug 1585760 - Store a flag in the TextureChild indicating use of the ImageBridge. r=sotaro — Details

This avoids calling GetTextureForwarder during shutdown which may return a null pointer.

Nicolas Silva [:nical]

Assignee

Comment 7

•

3 years ago

Attached file Bug 1585760 - Check that GetTextureForwarder isn't null in more places. r=sotaro — Details

Depends on D51615

Nicolas Silva [:nical]

Assignee

Comment 8

•

3 years ago

Comment on attachment 9100885 [details]
Bug 1585760 - Check that IPC is open before accessing parent protocol. r=sotaro

Security Approval Request

How easily could an exploit be constructed based on the patch?: Hard. It's a shutdown bug that relies on destruction order that isn't predictable.
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
Which older supported branches are affected by this flaw?: All
If not all supported branches, which bug introduced the flaw?: None
Do you have backports for the affected branches?: No
If not, how different, hard to create, and risky will they be?: The patches will probably apply without conflicts.
How likely is this patch to cause regressions; how much testing does it need?: Safe enough for Nightly. I would rather let it bake for a bit before thinking about uplifting as it may cause null-pointer crashes here and there.
No need for testing. We just need to let it bake for a while and look out for nullptr shutdown crashes.

Attachment #9100885 - Flags: sec-approval?

Nicolas Silva [:nical]

Assignee

Updated

•

3 years ago

Attachment #9106177 - Flags: sec-approval?

Attachment #9106178 - Flags: sec-approval?

Tom Ritter [:tjr]

Updated

•

3 years ago

Attachment #9100885 - Flags: sec-approval? → sec-approval+

Tom Ritter [:tjr]

Updated

•

3 years ago

Attachment #9106177 - Flags: sec-approval? → sec-approval+

Tom Ritter [:tjr]

Comment 9

•

3 years ago

Comment on attachment 9106178 [details]
Bug 1585760 - Check that GetTextureForwarder isn't null in more places. r=sotaro

Okay to land

Attachment #9106178 - Flags: sec-approval? → sec-approval+

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 10

•

3 years ago

https://hg.mozilla.org/integration/autoland/rev/1e074d517e70428a8bad2ca2c74200f42515d69b
https://hg.mozilla.org/integration/autoland/rev/892ec9a8534a5481e51dc9d169f35e1f132ce391
https://hg.mozilla.org/integration/autoland/rev/26df551013d3563a00d3be94a725196c43ac0306
https://hg.mozilla.org/mozilla-central/rev/1e074d517e70
https://hg.mozilla.org/mozilla-central/rev/892ec9a8534a
https://hg.mozilla.org/mozilla-central/rev/26df551013d3

Group: gfx-core-security → core-security-release

Status: NEW → RESOLVED

Closed: 3 years ago

status-firefox72: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla72

Ryan VanderMeulen [:RyanVM]

Updated

•

3 years ago

status-firefox70: --- → wontfix

status-firefox-esr68: --- → affected

tracking-firefox71: --- → +

tracking-firefox72: --- → +

tracking-firefox-esr68: --- → 71+

Release mgmt bot [:suhaib / :marco/ :calixte]

Comment 11

•

3 years ago

Changing the priority to p1 as the bug is tracked by a release manager for the current beta.
See What Do You Triage for more information

Priority: P3 → P1

Ryan VanderMeulen [:RyanVM]

Comment 12

•

3 years ago

Does this need Beta and ESR68 approval requests?

Flags: needinfo?(nical.bugzilla)

Nicolas Silva [:nical]

Assignee

Comment 13

•

3 years ago

Comment on attachment 9100885 [details]
Bug 1585760 - Check that IPC is open before accessing parent protocol. r=sotaro

Beta/Release Uplift Approval Request

User impact if declined: Potential shutdown UAF
Is this code covered by automated tests?: No
Has the fix been verified in Nightly?: Yes
Needs manual test from QE?: No
If yes, steps to reproduce:
List of other uplifts needed: None
Risk to taking this patch: Medium
Why is the change risky/not risky? (and alternatives if risky): The patch has been in nightly for two weeks without making waves. The shutdown logic is complicated so it gets a "medium" on the other hand time shutdown crashes don't tend to ruin the user experience as much as other crashes.
No risk of causing security issues (nullptr crahes at worst).
String changes made/needed:

ESR Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: potential shutdown UAF.
Fix Landed on Version: 72
Risk to taking this patch: Medium
Why is the change risky/not risky? (and alternatives if risky):
String or UUID changes made by this patch:

Flags: needinfo?(nical.bugzilla)

Attachment #9100885 - Flags: approval-mozilla-esr68?

Attachment #9100885 - Flags: approval-mozilla-beta?

Nicolas Silva [:nical]

Assignee

Updated

•

3 years ago

Attachment #9106177 - Flags: approval-mozilla-esr68?

Attachment #9106178 - Flags: approval-mozilla-esr68?

Nicolas Silva [:nical]

Assignee

Comment 14

•

3 years ago

Bugzilla gave me an error about setting the flags but the uplift requests apply to the other two patches as well.

Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)

Comment 15

•

3 years ago

Pascal, I'm ok with this for ESR if you'll take it for the 71 RC since it's had some bake time without bad results.

Flags: needinfo?(pascalc)

Pascal Chevrel:pascalc

Comment 16

•

3 years ago

Comment on attachment 9100885 [details]
Bug 1585760 - Check that IPC is open before accessing parent protocol. r=sotaro

Approved for RC

Flags: needinfo?(pascalc)

Attachment #9100885 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)

Updated

•

3 years ago

Attachment #9100885 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+

Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)

Updated

•

3 years ago

Attachment #9106177 - Flags: approval-mozilla-esr68?

Attachment #9106177 - Flags: approval-mozilla-esr68+

Attachment #9106177 - Flags: approval-mozilla-beta+

Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)

Updated

•

3 years ago

Attachment #9106178 - Flags: approval-mozilla-esr68?

Attachment #9106178 - Flags: approval-mozilla-esr68+

Attachment #9106178 - Flags: approval-mozilla-beta+

Ryan VanderMeulen [:RyanVM]

Comment 17

•

3 years ago

uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/28a8c133f68b42e58eac2fe56f5a5e7786e4560e
https://hg.mozilla.org/releases/mozilla-beta/rev/5ad3d109623247bc655f3018f1687fb1a64566d0
https://hg.mozilla.org/releases/mozilla-beta/rev/c12729e7c41f81f24d9581b7b1bf8c20cc485526

https://hg.mozilla.org/releases/mozilla-esr68/rev/234962fb9a22aaba57f34c0e8d751341665144e3
https://hg.mozilla.org/releases/mozilla-esr68/rev/c384f2be41a76410a75ba3ef9c6fbe96aef4407d
https://hg.mozilla.org/releases/mozilla-esr68/rev/3be8bbbf737d65d942e43a147d92d7be404d18ac

status-firefox71: affected → fixed

status-firefox-esr68: affected → fixed

Tom Ritter [:tjr]

Updated

•

3 years ago

Whiteboard: [adv-main71+r]

Tom Ritter [:tjr]

Updated

•

3 years ago

Whiteboard: [adv-main71+r] → [adv-main71+r][adv-esr68.3+r]

Sylvestre Ledru [:Sylvestre]

Updated

•

3 years ago

Blocks: asan-maintenance

Daniel Veditz [:dveditz]

Updated

•

3 years ago

Group: core-security-release

Bug 1585760 - Check that IPC is open before accessing parent protocol. r=sotaro 4 years ago Nicolas Silva [:nical] 47 bytes, text/x-phabricator-request	pascalc : approval-mozilla-beta+ lizzard : approval-mozilla-esr68+ tjr : sec-approval+	Details \| Review
Bug 1585760 - Store a flag in the TextureChild indicating use of the ImageBridge. r=sotaro 3 years ago Nicolas Silva [:nical] 47 bytes, text/x-phabricator-request	lizzard : approval-mozilla-beta+ lizzard : approval-mozilla-esr68+ tjr : sec-approval+	Details \| Review
Bug 1585760 - Check that GetTextureForwarder isn't null in more places. r=sotaro 3 years ago Nicolas Silva [:nical] 47 bytes, text/x-phabricator-request	lizzard : approval-mozilla-beta+ lizzard : approval-mozilla-esr68+ tjr : sec-approval+	Details \| Review