Closed Bug 1529027 Opened 5 years ago Closed 5 years ago

heap-use-after-free in [@ mozilla::layers::WebRenderImageHost::UseTextureHost]

Categories

(Core :: Graphics: WebRender, defect, P3)

Product:
Core
Component:
Graphics: WebRender
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla67
Tracking Flags:
Tracking Status
firefox-esr60 --- disabled
firefox65 --- disabled
firefox66 + fixed
firefox67 + fixed

People

(Reporter: tsmith, Assigned: sotaro)

References

(Blocks 2 open bugs)

Details

(Keywords: crash, csectype-uaf, sec-high, Whiteboard: [post-critsmash-triage])

Attachments

(2 files, 1 obsolete file)

Bug 1529027 - Make WebRenderImageHost to hold WeakPtr<WebRenderBridgeParent>
(deleted), text/x-phabricator-request
abillings
: sec-approval+
Details
patch for beta - Make WebRenderImageHost to hold WeakPtr<WebRenderBridgeParent>
(deleted), patch
RyanVM
: approval-mozilla-beta+
Details | Diff | Splinter Review

This bug was found while fuzzing m-c 20190219-ee6e77950205. The test case does not reproducible the bug. A reduced test case will be attached if one becomes available.

==2512==ERROR: AddressSanitizer: heap-use-after-free on address 0x61700048b668 at pc 0x7f9bcfd1a060 bp 0x7f9bae8718d0 sp 0x7f9bae8718c8
READ of size 8 at 0x61700048b668 thread T61 (Compositor)
    #0 0x7f9bcfd1a05f in get src/gfx/layers/../../mfbt/RefPtr.h:267:27
    #1 0x7f9bcfd1a05f in CompositorScheduler src/obj-firefox/dist/include/mozilla/layers/WebRenderBridgeParent.h:72
    #2 0x7f9bcfd1a05f in mozilla::layers::WebRenderImageHost::UseTextureHost(nsTArray<mozilla::layers::CompositableHost::TimedTexture> const&) src/gfx/layers/wr/WebRenderImageHost.cpp:69
    #3 0x7f9bd007d068 in mozilla::layers::CompositableParentManager::ReceiveCompositableUpdate(mozilla::layers::CompositableOperationDetail const&, mozilla::NotNull<mozilla::layers::CompositableHost*>) src/gfx/layers/ipc/CompositableTransactionParent.cpp:179:24
    #4 0x7f9bd00ed6da in mozilla::layers::ImageBridgeParent::RecvUpdate(nsTArray<mozilla::layers::CompositableOperation>&&, nsTArray<mozilla::layers::OpDestroy>&&, unsigned long const&) src/gfx/layers/ipc/ImageBridgeParent.cpp:186:10
    #5 0x7f9bce21f5fa in mozilla::layers::PImageBridgeParent::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PImageBridgeParent.cpp:264:61
    #6 0x7f9bcdb9bd79 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2150:21
    #7 0x7f9bcdb97b7a in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2077:9
    #8 0x7f9bcdb99d81 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1936:3
    #9 0x7f9bcdb9ab47 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1967:13
    #10 0x7f9bcda90e6f in RunTask src/ipc/chromium/src/base/message_loop.cc:442:9
    #11 0x7f9bcda90e6f in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask&&) src/ipc/chromium/src/base/message_loop.cc:450
    #12 0x7f9bcda9236b in MessageLoop::DoWork() src/ipc/chromium/src/base/message_loop.cc:523:13
    #13 0x7f9bcda94dd4 in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) src/ipc/chromium/src/base/message_pump_default.cc:35:31
    #14 0x7f9bcda8f29e in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #15 0x7f9bcda8f29e in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
    #16 0x7f9bcda8f29e in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #17 0x7f9bcdadaad5 in base::Thread::ThreadMain() src/ipc/chromium/src/base/thread.cc:192:16
    #18 0x7f9bcdaa71d8 in ThreadFunc(void*) src/ipc/chromium/src/base/platform_thread_posix.cc:40:13
    #19 0x7f9bf1d6a6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    #20 0x7f9bf0de741c in clone /build/glibc-Cl5G7W/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

0x61700048b668 is located 232 bytes inside of 696-byte region [0x61700048b580,0x61700048b838)
freed by thread T61 (Compositor) here:
    #0 0x56151a5d6a22 in __interceptor_free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x7f9bd00cf541 in Release src/gfx/layers/ipc/ISurfaceAllocator.h:71:3
    #2 0x7f9bd00cf541 in mozilla::layers::CrossProcessCompositorBridgeParent::DeallocPWebRenderBridgeParent(mozilla::layers::PWebRenderBridgeParent*) src/gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp:273
    #3 0x7f9bcde310c4 in mozilla::layers::PCompositorBridgeParent::RemoveManagee(int, mozilla::ipc::IProtocol*) src/obj-firefox/ipc/ipdl/PCompositorBridgeParent.cpp
    #4 0x7f9bce7c3f37 in mozilla::layers::PWebRenderBridgeParent::Send__delete__(mozilla::layers::PWebRenderBridgeParent*) src/obj-firefox/ipc/ipdl/PWebRenderBridgeParent.cpp:121:12
    #5 0x7f9bcfcbec20 in mozilla::layers::WebRenderBridgeParent::HandleShutdown() src/gfx/layers/wr/WebRenderBridgeParent.cpp:353:8
    #6 0x7f9bce7c7d85 in mozilla::layers::PWebRenderBridgeParent::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PWebRenderBridgeParent.cpp:933:20
    #7 0x7f9bcde539d7 in mozilla::layers::PCompositorManagerParent::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PCompositorManagerParent.cpp:111:28
    #8 0x7f9bcdb9bd79 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2150:21
    #9 0x7f9bcdb97b7a in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2077:9
    #10 0x7f9bcdb99d81 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1936:3
    #11 0x7f9bcdb9ab47 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1967:13
    #12 0x7f9bcda90e6f in RunTask src/ipc/chromium/src/base/message_loop.cc:442:9
    #13 0x7f9bcda90e6f in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask&&) src/ipc/chromium/src/base/message_loop.cc:450
    #14 0x7f9bcda9236b in MessageLoop::DoWork() src/ipc/chromium/src/base/message_loop.cc:523:13
    #15 0x7f9bcda94dd4 in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) src/ipc/chromium/src/base/message_pump_default.cc:35:31
    #16 0x7f9bcda8f29e in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #17 0x7f9bcda8f29e in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
    #18 0x7f9bcda8f29e in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #19 0x7f9bcdadaad5 in base::Thread::ThreadMain() src/ipc/chromium/src/base/thread.cc:192:16
    #20 0x7f9bcdaa71d8 in ThreadFunc(void*) src/ipc/chromium/src/base/platform_thread_posix.cc:40:13
    #21 0x7f9bf1d6a6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

previously allocated by thread T61 (Compositor) here:
    #0 0x56151a5d6da3 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
    #1 0x56151a60b63d in moz_xmalloc src/memory/mozalloc/mozalloc.cpp:68:15
    #2 0x7f9bd00cea1b in operator new src/obj-firefox/dist/include/mozilla/mozalloc.h:131:10
    #3 0x7f9bd00cea1b in mozilla::layers::CrossProcessCompositorBridgeParent::AllocPWebRenderBridgeParent(mozilla::wr::PipelineId const&, mozilla::gfx::IntSizeTyped<mozilla::LayoutDevicePixel> const&) src/gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp:250
    #4 0x7f9bcde3219f in mozilla::layers::PCompositorBridgeParent::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PCompositorBridgeParent.cpp:1142:71
    #5 0x7f9bcde539d7 in mozilla::layers::PCompositorManagerParent::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PCompositorManagerParent.cpp:111:28
    #6 0x7f9bcdb9bd79 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2150:21
    #7 0x7f9bcdb97b7a in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2077:9
    #8 0x7f9bcdb99d81 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1936:3
    #9 0x7f9bcdb9ab47 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1967:13
    #10 0x7f9bcda90e6f in RunTask src/ipc/chromium/src/base/message_loop.cc:442:9
    #11 0x7f9bcda90e6f in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask&&) src/ipc/chromium/src/base/message_loop.cc:450
    #12 0x7f9bcda9236b in MessageLoop::DoWork() src/ipc/chromium/src/base/message_loop.cc:523:13
    #13 0x7f9bcda94dd4 in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) src/ipc/chromium/src/base/message_pump_default.cc:35:31
    #14 0x7f9bcda8f29e in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #15 0x7f9bcda8f29e in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
    #16 0x7f9bcda8f29e in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #17 0x7f9bcdadaad5 in base::Thread::ThreadMain() src/ipc/chromium/src/base/thread.cc:192:16
    #18 0x7f9bcdaa71d8 in ThreadFunc(void*) src/ipc/chromium/src/base/platform_thread_posix.cc:40:13
    #19 0x7f9bf1d6a6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Thread T61 (Compositor) created by T0 here:
    #0 0x56151a5bf6bd in __interceptor_pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
    #1 0x7f9bcdaa3b72 in CreateThread src/ipc/chromium/src/base/platform_thread_posix.cc:123:14
    #2 0x7f9bcdaa3b72 in PlatformThread::Create(unsigned long, PlatformThread::Delegate*, unsigned long*) src/ipc/chromium/src/base/platform_thread_posix.cc:134
    #3 0x7f9bcdad9eaf in base::Thread::StartWithOptions(base::Thread::Options const&) src/ipc/chromium/src/base/thread.cc:97:8
    #4 0x7f9bd00c7a0a in CreateCompositorThread src/gfx/layers/ipc/CompositorThread.cpp:92:26
    #5 0x7f9bd00c7a0a in mozilla::layers::CompositorThreadHolder::CompositorThreadHolder() src/gfx/layers/ipc/CompositorThread.cpp:45
    #6 0x7f9bd00c816a in mozilla::layers::CompositorThreadHolder::Start() src/gfx/layers/ipc/CompositorThread.cpp:113:33
    #7 0x7f9bd01e73f0 in gfxPlatform::Init() src/gfx/thebes/gfxPlatform.cpp:973:3
    #8 0x7f9bd01e4863 in gfxPlatform::GetPlatform() src/gfx/thebes/gfxPlatform.cpp:497:5
    #9 0x7f9bd6d76818 in mozilla::widget::GfxInfoBase::GetContentBackend(nsTSubstring<char16_t>&) src/widget/GfxInfoBase.cpp:1481:25
    #10 0x7f9bcc94c171 in NS_InvokeByIndex src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:106
    #11 0x7f9bced39337 in Invoke src/js/xpconnect/src/XPCWrappedNative.cpp:1638:10
    #12 0x7f9bced39337 in Call src/js/xpconnect/src/XPCWrappedNative.cpp:1186
    #13 0x7f9bced39337 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) src/js/xpconnect/src/XPCWrappedNative.cpp:1152
    #14 0x7f9bced41eef in GetAttribute src/js/xpconnect/src/xpcprivate.h:1477:12
    #15 0x7f9bced41eef in XPC_WN_GetterSetter(JSContext*, unsigned int, JS::Value*) src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:986
    #16 0x7f9bdbc57507 in CallJSNative src/js/src/vm/Interpreter.cpp:440:13
    #17 0x7f9bdbc57507 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:532
    #18 0x7f9bdbc5bdc0 in InternalCall src/js/src/vm/Interpreter.cpp:587:10
    #19 0x7f9bdbc5bdc0 in Call src/js/src/vm/Interpreter.cpp:603
    #20 0x7f9bdbc5bdc0 in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) src/js/src/vm/Interpreter.cpp:727
    #21 0x7f9bdc276038 in CallGetter src/js/src/vm/NativeObject.cpp:2232:12
    #22 0x7f9bdc276038 in GetExistingProperty<js::CanGC> src/js/src/vm/NativeObject.cpp:2284
    #23 0x7f9bdc276038 in NativeGetPropertyInline<js::CanGC> src/js/src/vm/NativeObject.cpp:2533
    #24 0x7f9bdc276038 in js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<JS::PropertyKey>, JS::MutableHandle<JS::Value>) src/js/src/vm/NativeObject.cpp:2570
    #25 0x7f9bdbc3d855 in GetProperty src/js/src/vm/ObjectOperations-inl.h:117:10
    #26 0x7f9bdbc3d855 in GetObjectElementOperation src/js/src/vm/Interpreter-inl.h:496
    #27 0x7f9bdbc3d855 in GetElementOperation src/js/src/vm/Interpreter-inl.h:612
    #28 0x7f9bdbc3d855 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:2872
    #29 0x7f9bdbc220a8 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:420:10
    #30 0x7f9bdbc57e76 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:560:13
    #31 0x7f9bdbc59ac2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) src/js/src/vm/Interpreter.cpp:603:8
    #32 0x7f9bdc81ef9a in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/jsapi.cpp:2560:10
    #33 0x7f9bced1e542 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) src/js/xpconnect/src/XPCWrappedJSClass.cpp:988:17
    #34 0x7f9bcc94d878 in PrepareAndDispatch src/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:127:37
    #35 0x7f9bcc94c74a in SharedStub (/home/ubuntu/firefox/libxul.so+0x4a6374a)
    #36 0x7f9bcc8a53c9 in NS_CreateServicesFromCategory(char const*, nsISupports*, char const*, char16_t const*) src/xpcom/components/nsCategoryManager.cpp:677:19
    #37 0x7f9bdb996560 in nsXREDirProvider::DoStartup() src/toolkit/xre/nsXREDirProvider.cpp:1019:11
    #38 0x7f9bdb96b6de in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4532:16
    #39 0x7f9bdb96ef79 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4842:8
    #40 0x7f9bdb9709c3 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4926:21
    #41 0x56151a60968c in do_main src/browser/app/nsBrowserApp.cpp:214:22
    #42 0x56151a60968c in main src/browser/app/nsBrowserApp.cpp:293
    #43 0x7f9bf0d0082f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291

Looks like mWrBridge was not cleared before the underlying object got released?

Blocks: stage-wr-trains
Priority: -- → P3

Any thoughts on what could be happening here?

Flags: needinfo?(sotaro.ikeda.g)
Assignee: nobody → sotaro.ikeda.g
Blocks: wr-fuzz

WebRenderImageHost::ClearWrBridge() expects that same WebRenderBridgeParent is set by WebRenderImageHost::SetWrBridge(), but there is a case that different WebRenderBridgeParent is set by the SetWrBridge(). One example is disable e10s and move a tab between windows.

Blocks: 1363347
Flags: needinfo?(sotaro.ikeda.g)

It seems simpler just to use WeakPtr to address the problem.

Please remember to follow the security bug approval process: https://bugzilla.mozilla.org/show_bug.cgi?id=1528621

Keywords: sec-high

Comment on attachment 9045134 [details]
Bug 1529027 - Make WebRenderImageHost to hold WeakPtr<WebRenderBridgeParent>

Security Approval Request

How easily could an exploit be constructed based on the patch?

I is not easy. We do not know a STR to reproduce the problem.

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?

Firefox beta(66)

If not all supported branches, which bug introduced the flaw?

Bug 1363347

Do you have backports for the affected branches?

No

If not, how different, hard to create, and risky will they be?

It is very easy to create the patch. It will not be risky.

How likely is this patch to cause regressions; how much testing does it need?

It seems not cause the regression. It change the pointer to WeakPtr<WebRenderBridgeParent>. WebRenderImageHost already has nullptr check to the pointer access.
The flaw was introduced at Firefox 55, but it affect only to Firefox beta(66) and nightly(67), since the code path is used only when WebRender is enabled.

Attachment #9045134 - Flags: sec-approval?

sec-approval+ for trunk. Please nominate a Beta branch patch as well.

status-firefox-esr60: --- → disabled
tracking-firefox66: --- → +
tracking-firefox67: --- → +
Attachment #9045134 - Flags: sec-approval? → sec-approval+

https://hg.mozilla.org/mozilla-central/rev/0e0b4b2f2973

Please add an approval request to that Beta patch when you get a chance.

Group: gfx-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox67: affected → fixed
Flags: needinfo?(sotaro.ikeda.g)
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Comment on attachment 9046080 [details] [diff] [review]
patch for beta - Make WebRenderImageHost to hold WeakPtr<WebRenderBridgeParent>

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1363347
  • User impact if declined: Firefox might crash during playing video.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It changed the raw pointer to WeakPtr<WebRenderBridgeParent>. WebRenderImageHost already has nullptr check to the pointer access.
  • String changes made/needed: None
Flags: needinfo?(sotaro.ikeda.g)
Attachment #9046080 - Flags: approval-mozilla-beta?
Comment on attachment 9046080 [details] [diff] [review]
patch for beta - Make WebRenderImageHost to hold WeakPtr<WebRenderBridgeParent>

Crash fix, OK for beta uplift.
Attachment #9046080 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

The beta patch attached here seems to be for bug 1516834. Sotaro, can you access what needs to be done, please?

Flags: needinfo?(sotaro.ikeda.g)

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #17)

The beta patch attached here seems to be for bug 1516834. Sotaro, can you access what needs to be done, please?

Sorry, I attached different patch. I am going to update correct patch soon.

Flags: needinfo?(sotaro.ikeda.g)
Attachment #9046080 - Flags: approval-mozilla-beta+
Comment on attachment 9046347 [details] [diff] [review]
patch for beta - Make WebRenderImageHost to hold WeakPtr<WebRenderBridgeParent>

Moving the approval over to the updated patch for posterity's sake.
Attachment #9046347 - Flags: approval-mozilla-beta+
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Group: core-security-release
Regressions: 1676505
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: