Closed Bug 1538736 Opened 4 years ago Closed 4 years ago

AddressSanitizer: heap-use-after-free [@ Manager] with READ of size 8

Categories

(Core :: Graphics: Layers, defect, P2)

Product:
Core
Component:
Graphics: Layers
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox-esr60 67+ fixed
firefox66 --- wontfix
firefox67 + fixed
firefox68 + fixed

People

(Reporter: jkratzer, Assigned: nical)

References

(Blocks 2 open bugs)

Details

(4 keywords, Whiteboard: [adv-main67+][adv-esr60.7+])

Attachments

(1 file)

Bug 1538736 - Make extra sure Compositables don't refer back to layers after reassignment. r=sotaro
47 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-beta+
lizzard
: approval-mozilla-esr60+
abillings
: sec-approval+
Details | Review

Found while fuzzing mozilla-central rev fd1adb9941e9 (20190322) on Linux x64. I don't currently have a working testcase but will update this if one becomes available.

==680==ERROR: AddressSanitizer: heap-use-after-free on address 0x6190014a2d98 at pc 0x7eff77a1c7d1 bp 0x7eff54004790 sp 0x7eff54004788
READ of size 8 at 0x6190014a2d98 thread T38 (Compositor)
#0 0x7eff77a1c7d0 in Manager src/obj-firefox/dist/include/Layers.h:831:36
#1 0x7eff77a1c7d0 in mozilla::layers::CompositableHost::GetLayerManager() const src/gfx/layers/composite/CompositableHost.cpp:155
#2 0x7eff77a7d3b8 in mozilla::layers::ImageHost::UseTextureHost(nsTArray<mozilla::layers::CompositableHost::TimedTexture> const&) src/gfx/layers/composite/ImageHost.cpp:72:26
#3 0x7eff77adb038 in mozilla::layers::CompositableParentManager::ReceiveCompositableUpdate(mozilla::layers::CompositableOperationDetail const&, mozilla::NotNull<mozilla::layers::CompositableHost*>) src/gfx/layers/ipc/CompositableTransactionParent.cpp:179:24
#4 0x7eff77b4defd in mozilla::layers::ImageBridgeParent::RecvUpdate(nsTArray<mozilla::layers::CompositableOperation>&&, nsTArray<mozilla::layers::OpDestroy>&&, unsigned long const&) src/gfx/layers/ipc/ImageBridgeParent.cpp:191:10
#5 0x7eff75c3a33a in mozilla::layers::PImageBridgeParent::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PImageBridgeParent.cpp:260:61
#6 0x7eff755c02b9 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2151:21
#7 0x7eff755bbffa in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2078:9
#8 0x7eff755be237 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1937:3
#9 0x7eff755befc7 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1968:13
#10 0x7eff754a0d9f in RunTask src/ipc/chromium/src/base/message_loop.cc:442:9
#11 0x7eff754a0d9f in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask&&) src/ipc/chromium/src/base/message_loop.cc:450
#12 0x7eff754a229b in MessageLoop::DoWork() src/ipc/chromium/src/base/message_loop.cc:523:13
#13 0x7eff754a4d04 in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) src/ipc/chromium/src/base/message_pump_default.cc:35:31
#14 0x7eff7549f1ce in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
#15 0x7eff7549f1ce in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
#16 0x7eff7549f1ce in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
#17 0x7eff754e9047 in base::Thread::ThreadMain() src/ipc/chromium/src/base/thread.cc:192:16
#18 0x7eff754b7248 in ThreadFunc(void*) src/ipc/chromium/src/base/platform_thread_posix.cc:40:13
#19 0x7eff992fc6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
#20 0x7eff9837941c in clone /build/glibc-Cl5G7W/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

0x6190014a2d98 is located 24 bytes inside of 1056-byte region [0x6190014a2d80,0x6190014a31a0)
freed by thread T38 (Compositor) here:
#0 0x563a631609e2 in free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
#1 0x7eff7755615e in Release src/obj-firefox/dist/include/Layers.h:808:3
#2 0x7eff7755615e in mozilla::layers::ContainerLayer::RemoveAllChildren() src/gfx/layers/Layers.cpp:866
#3 0x7eff77a22444 in ~ContainerLayerComposite src/gfx/layers/composite/ContainerLayerComposite.cpp:687:3
#4 0x7eff77a22444 in mozilla::layers::ContainerLayerComposite::~ContainerLayerComposite() src/gfx/layers/composite/ContainerLayerComposite.cpp:675
#5 0x7eff775de6fb in ~ContainerLayerProperties src/gfx/layers/LayerTreeInvalidation.cpp:338:8
#6 0x7eff775de6fb in mozilla::layers::ContainerLayerProperties::~ContainerLayerProperties() src/gfx/layers/LayerTreeInvalidation.cpp:338
#7 0x7eff775de59a in operator() src/gfx/layers/../../mfbt/UniquePtr.h:486:5
#8 0x7eff775de59a in reset src/gfx/layers/../../mfbt/UniquePtr.h:323
#9 0x7eff775de59a in ~UniquePtr src/gfx/layers/../../mfbt/UniquePtr.h:274
#10 0x7eff775de59a in Destruct src/obj-firefox/dist/include/nsTArray.h:522
#11 0x7eff775de59a in DestructRange src/obj-firefox/dist/include/nsTArray.h:2178
#12 0x7eff775de59a in ClearAndRetainStorage src/obj-firefox/dist/include/nsTArray.h:1296
#13 0x7eff775de59a in ~nsTArray_Impl src/obj-firefox/dist/include/nsTArray.h:877
#14 0x7eff775de59a in ~ContainerLayerProperties src/gfx/layers/LayerTreeInvalidation.cpp:338
#15 0x7eff775de59a in mozilla::layers::ContainerLayerProperties::~ContainerLayerProperties() src/gfx/layers/LayerTreeInvalidation.cpp:338
#16 0x7eff775de59a in operator() src/gfx/layers/../../mfbt/UniquePtr.h:486:5
#17 0x7eff775de59a in reset src/gfx/layers/../../mfbt/UniquePtr.h:323
#18 0x7eff775de59a in ~UniquePtr src/gfx/layers/../../mfbt/UniquePtr.h:274
#19 0x7eff775de59a in Destruct src/obj-firefox/dist/include/nsTArray.h:522
#20 0x7eff775de59a in DestructRange src/obj-firefox/dist/include/nsTArray.h:2178
#21 0x7eff775de59a in ClearAndRetainStorage src/obj-firefox/dist/include/nsTArray.h:1296
#22 0x7eff775de59a in ~nsTArray_Impl src/obj-firefox/dist/include/nsTArray.h:877
#23 0x7eff775de59a in ~ContainerLayerProperties src/gfx/layers/LayerTreeInvalidation.cpp:338
#24 0x7eff775de59a in mozilla::layers::ContainerLayerProperties::~ContainerLayerProperties() src/gfx/layers/LayerTreeInvalidation.cpp:338
#25 0x7eff77a989dc in operator() src/gfx/layers/../../mfbt/UniquePtr.h:486:5
#26 0x7eff77a989dc in reset src/gfx/layers/../../mfbt/UniquePtr.h:323
#27 0x7eff77a989dc in operator= src/gfx/layers/../../mfbt/UniquePtr.h:277
#28 0x7eff77a989dc in mozilla::layers::LayerManagerComposite::UpdateAndRender() src/gfx/layers/composite/LayerManagerComposite.cpp:524
#29 0x7eff77a9645d in mozilla::layers::LayerManagerComposite::EndTransaction(mozilla::TimeStamp const&, mozilla::layers::LayerManager::EndTransactionFlags) src/gfx/layers/composite/LayerManagerComposite.cpp:440:5
#30 0x7eff77b06ff1 in mozilla::layers::CompositorBridgeParent::CompositeToTarget(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::gfx::DrawTarget*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const*) src/gfx/layers/ipc/CompositorBridgeParent.cpp:1007:18
#31 0x7eff77b28d58 in mozilla::layers::CompositorVsyncScheduler::Composite(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/gfx/layers/ipc/CompositorVsyncScheduler.cpp:229:27
#32 0x7eff77b7c7ab in applyImpl<mozilla::layers::CompositorVsyncScheduler, void (mozilla::layers::CompositorVsyncScheduler::)(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp), StoreCopyPassByConstLRef<mozilla::layers::BaseTransactionId<mozilla::VsyncIdType> >, StoreCopyPassByConstLRef<mozilla::TimeStamp> , 0, 1> src/obj-firefox/dist/include/nsThreadUtils.h:1122:12
#33 0x7eff77b7c7ab in apply<mozilla::layers::CompositorVsyncScheduler, void (mozilla::layers::CompositorVsyncScheduler::)(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp)> src/obj-firefox/dist/include/nsThreadUtils.h:1128
#34 0x7eff77b7c7ab in mozilla::detail::RunnableMethodImpl<mozilla::layers::CompositorVsyncScheduler*, void (mozilla::layers::CompositorVsyncScheduler::)(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp), true, (mozilla::RunnableKind)1, mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1174
#35 0x7eff754a0d9f in RunTask src/ipc/chromium/src/base/message_loop.cc:442:9
#36 0x7eff754a0d9f in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask&&) src/ipc/chromium/src/base/message_loop.cc:450
#37 0x7eff754a229b in MessageLoop::DoWork() src/ipc/chromium/src/base/message_loop.cc:523:13
#38 0x7eff754a4d04 in base::MessagePumpDefault::Run(base::MessagePump::Delegate) src/ipc/chromium/src/base/message_pump_default.cc:35:31
#39 0x7eff7549f1ce in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
#40 0x7eff7549f1ce in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
#41 0x7eff7549f1ce in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
#42 0x7eff754e9047 in base::Thread::ThreadMain() src/ipc/chromium/src/base/thread.cc:192:16
#43 0x7eff754b7248 in ThreadFunc(void*) src/ipc/chromium/src/base/platform_thread_posix.cc:40:13
#44 0x7eff992fc6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

previously allocated by thread T38 (Compositor) here:
#0 0x563a63160d63 in __interceptor_malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
#1 0x563a631955fd in moz_xmalloc src/memory/mozalloc/mozalloc.cpp:68:15
#2 0x7eff77aa3a98 in operator new src/obj-firefox/dist/include/mozilla/mozalloc.h:131:10
#3 0x7eff77aa3a98 in mozilla::layers::LayerManagerComposite::CreateImageLayer() src/gfx/layers/composite/LayerManagerComposite.cpp:1233
#4 0x7eff77b54a6e in mozilla::layers::LayerTransactionParent::RecvUpdate(mozilla::layers::TransactionInfo const&) src/gfx/layers/ipc/LayerTransactionParent.cpp:216:51
#5 0x7eff75c9f43e in mozilla::layers::PLayerTransactionParent::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PLayerTransactionParent.cpp:108:66
#6 0x7eff7587b4d7 in mozilla::layers::PCompositorManagerParent::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PCompositorManagerParent.cpp:107:28
#7 0x7eff755c02b9 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2151:21
#8 0x7eff755bbffa in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2078:9
#9 0x7eff755be237 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1937:3
#10 0x7eff755befc7 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1968:13
#11 0x7eff754a0d9f in RunTask src/ipc/chromium/src/base/message_loop.cc:442:9
#12 0x7eff754a0d9f in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask&&) src/ipc/chromium/src/base/message_loop.cc:450
#13 0x7eff754a229b in MessageLoop::DoWork() src/ipc/chromium/src/base/message_loop.cc:523:13
#14 0x7eff754a4d04 in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) src/ipc/chromium/src/base/message_pump_default.cc:35:31
#15 0x7eff7549f1ce in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
#16 0x7eff7549f1ce in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
#17 0x7eff7549f1ce in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
#18 0x7eff754e9047 in base::Thread::ThreadMain() src/ipc/chromium/src/base/thread.cc:192:16
#19 0x7eff754b7248 in ThreadFunc(void*) src/ipc/chromium/src/base/platform_thread_posix.cc:40:13
#20 0x7eff992fc6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Thread T38 (Compositor) created by T0 here:
#0 0x563a6314967d in pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
#1 0x7eff754b3a02 in CreateThread src/ipc/chromium/src/base/platform_thread_posix.cc:123:14
#2 0x7eff754b3a02 in PlatformThread::Create(unsigned long, PlatformThread::Delegate*, unsigned long*) src/ipc/chromium/src/base/platform_thread_posix.cc:134
#3 0x7eff754e8408 in base::Thread::StartWithOptions(base::Thread::Options const&) src/ipc/chromium/src/base/thread.cc:97:8
#4 0x7eff77b2622a in CreateCompositorThread src/gfx/layers/ipc/CompositorThread.cpp:90:26
#5 0x7eff77b2622a in mozilla::layers::CompositorThreadHolder::CompositorThreadHolder() src/gfx/layers/ipc/CompositorThread.cpp:42
#6 0x7eff77b26991 in mozilla::layers::CompositorThreadHolder::Start() src/gfx/layers/ipc/CompositorThread.cpp:111:33
#7 0x7eff77c4ce0a in gfxPlatform::Init() src/gfx/thebes/gfxPlatform.cpp:961:3
#8 0x7eff77c4a3d3 in gfxPlatform::GetPlatform() src/gfx/thebes/gfxPlatform.cpp:480:5
#9 0x7eff7e8a6c18 in mozilla::widget::GfxInfoBase::GetContentBackend(nsTSubstring<char16_t>&) src/widget/GfxInfoBase.cpp:1479:25
#10 0x7eff74320ba1 in NS_InvokeByIndex src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:106
#11 0x7eff7676a947 in Invoke src/js/xpconnect/src/XPCWrappedNative.cpp:1630:10
#12 0x7eff7676a947 in Call src/js/xpconnect/src/XPCWrappedNative.cpp:1178
#13 0x7eff7676a947 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) src/js/xpconnect/src/XPCWrappedNative.cpp:1144
#14 0x7eff767736ab in GetAttribute src/js/xpconnect/src/xpcprivate.h:1482:12
#15 0x7eff767736ab in XPC_WN_GetterSetter(JSContext*, unsigned int, JS::Value*) src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:981
#16 0x7eff831ddc07 in CallJSNative src/js/src/vm/Interpreter.cpp:442:13
#17 0x7eff831ddc07 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:534
#18 0x7eff831e24c0 in InternalCall src/js/src/vm/Interpreter.cpp:589:10
#19 0x7eff831e24c0 in Call src/js/src/vm/Interpreter.cpp:605
#20 0x7eff831e24c0 in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) src/js/src/vm/Interpreter.cpp:729
#21 0x7eff838215aa in CallGetter src/js/src/vm/NativeObject.cpp:2243:12
#22 0x7eff838215aa in GetExistingProperty<js::CanGC> src/js/src/vm/NativeObject.cpp:2295
#23 0x7eff838215aa in NativeGetPropertyInline<js::CanGC> src/js/src/vm/NativeObject.cpp:2544
#24 0x7eff838215aa 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:2581
#25 0x7eff831c37c2 in GetProperty src/js/src/vm/ObjectOperations-inl.h:117:10
#26 0x7eff831c37c2 in GetObjectElementOperation src/js/src/vm/Interpreter-inl.h:494
#27 0x7eff831c37c2 in GetElementOperation src/js/src/vm/Interpreter-inl.h:608
#28 0x7eff831c37c2 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:2883
#29 0x7eff831a80d8 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:422:10
#30 0x7eff831de576 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:562:13
#31 0x7eff831e01c2 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:605:8
#32 0x7eff83e013e7 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 0x7eff7674fb82 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) src/js/xpconnect/src/XPCWrappedJSClass.cpp:993:17
#34 0x7eff743222a8 in PrepareAndDispatch src/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:127:37
#35 0x7eff7432117a in SharedStub (/home/ubuntu/firefox/libxul.so+0x4a0917a)
#36 0x7eff74277be9 in NS_CreateServicesFromCategory(char const*, nsISupports*, char const*, char16_t const*) src/xpcom/components/nsCategoryManager.cpp:679:19
#37 0x7eff82f17160 in nsXREDirProvider::DoStartup() src/toolkit/xre/nsXREDirProvider.cpp:1019:11
#38 0x7eff82eecc9b in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4409:16
#39 0x7eff82ef04a8 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4727:8
#40 0x7eff82ef1d39 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4811:21
#41 0x563a6319364c in do_main src/browser/app/nsBrowserApp.cpp:212:22
#42 0x563a6319364c in main src/browser/app/nsBrowserApp.cpp:291
#43 0x7eff9829282f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291

SUMMARY: AddressSanitizer: heap-use-after-free src/obj-firefox/dist/include/Layers.h:831:36 in Manager
Shadow bytes around the buggy address:
0x0c328028c560: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c328028c570: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c328028c580: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c328028c590: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c328028c5a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c328028c5b0: fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd
0x0c328028c5c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c328028c5d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c328028c5e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c328028c5f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c328028c600: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
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
==680==ABORTING

Keywords: sec-high
Group: core-security → gfx-core-security

Does this bug look actionable, Matt?

Flags: needinfo?(matt.woodrow)
Priority: -- → P2

I think it might be solvable.

We don't have a testcase, but we do have the stacks.

It looks like the Compositable for ImageBridge is referencing a Layer that has been deleted. It looks like ImageLayerComposite::CleanupResources should be cleaning this up, but I guess there's a race condition where we fail.

Any ideas Nical?

Flags: needinfo?(matt.woodrow) → needinfo?(nical.bugzilla)

We will look at this in more detail later next week

Assignee: nobody → nical.bugzilla

This patch should fix the only ways I can see for a compositable to point to a dead layer.

Flags: needinfo?(nical.bugzilla)

Comment on attachment 9059017 [details]
Bug 1538736 - Make extra sure Compositables don't refer back to layers after reassignment. r=sotaro

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Hard. Very rare race condition which appears to only have been caught by fuzzing once without reproducible test case.
  • 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 of them probably.
  • 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?: It would be easy to backport the patch, but I wouldn't rush to uplift a fix considering how difficult it is to reproduce/exploit the issue. If anything I'd let the patch bake in nightly for a bit before uplifting.
  • How likely is this patch to cause regressions; how much testing does it need?: Rather unlikely. We can't test for it specifically because we can't reproduce the issue so far. It went through a try run without suspicious breakage.
Attachment #9059017 - Flags: sec-approval?
Attachment #9059017 - Flags: sec-approval? → sec-approval+

https://hg.mozilla.org/integration/autoland/rev/d3b41d3190e5

FYI, when we get to the point of uplifting, this grafts cleanly to both Beta and ESR60 as-landed.

https://hg.mozilla.org/mozilla-central/rev/d3b41d3190e5

Group: gfx-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox68: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Can you request uplift for beta and esr? Thanks.

Flags: needinfo?(nical.bugzilla)

Comment on attachment 9059017 [details]
Bug 1538736 - Make extra sure Compositables don't refer back to layers after reassignment. r=sotaro

Beta/Release Uplift Approval Request

  • User impact if declined: Potential user-after-free.
  • Is this code covered by automated tests?: No
  • 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): We don't know how to reproduce the bug so we couldn't verify the fix, but it's reasonably simple and has been on nightly for a few days without making waves.
  • String changes made/needed: None.

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined: Potential user-after-free crash.
  • Fix Landed on Version: 67
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): We don't know how to reproduce the bug so we couldn't verify the fix, but it's reasonably simple and has been on nightly for a few days without making waves.
  • String or UUID changes made by this patch: None.
Flags: needinfo?(nical.bugzilla)
Attachment #9059017 - Flags: approval-mozilla-esr60?
Attachment #9059017 - Flags: approval-mozilla-beta?

Comment on attachment 9059017 [details]
Bug 1538736 - Make extra sure Compositables don't refer back to layers after reassignment. r=sotaro

Uplift approved for 67 beta 15, thanks.

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

Comment on attachment 9059017 [details]
Bug 1538736 - Make extra sure Compositables don't refer back to layers after reassignment. r=sotaro

Fix for sec-high issue, seem ok in nightly; let's take it for esr 60.7.

Attachment #9059017 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Whiteboard: [adv-main67+][adv-esr60.7+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.