1725854 - IPC Parent Crash [@ wgpu_core::hub::Storage$LT$T$C$I$GT$::iter::_$u7b$$u7b$closure$u7d$$u7d$] with potential use-after-free

Status: RESOLVED FIXED

(Core :: Graphics: WebGPU, defect)

Description

[Christian Holler (:decoder)]

In experimental IPC fuzzing, we found the following crash on mozilla-central revision 160071ad9de0+ (patched fuzzing build):

=================================================================
==1687==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x7fffee5fa4a0 bp 0x7fffc6238e90 sp 0x7fffc6238dd0 T24)
==1687==The signal is caused by a READ memory access.
==1687==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
    #0 0x7fffee5fa4a0 in wgpu_core::hub::Storage$LT$T$C$I$GT$::iter::_$u7b$$u7b$closure$u7d$$u7d$::h93c625c5511300fa gfx/wgpu/wgpu-core/src/hub.rs:220:17
    #1 0x7fffee5fa4a0 in core::ops::function::impls::_$LT$impl$u20$core..ops..function..FnMut$LT$A$GT$$u20$for$u20$$RF$mut$u20$F$GT$::call_mut::hc26d543267d8236e /rustc/2f391da2e6ac73faa3570b79de239fd8c0edf1a9/library/core/src/ops/function.rs:269:13
    #2 0x7fffee5fa4a0 in core::iter::traits::iterator::Iterator::find_map::check::_$u7b$$u7b$closure$u7d$$u7d$::h6f03db4db5d39338 /rustc/2f391da2e6ac73faa3570b79de239fd8c0edf1a9/library/core/src/iter/traits/iterator.rs:2410:32
    #3 0x7fffee5fa4a0 in _$LT$core..iter..adapters..enumerate..Enumerate$LT$I$GT$$u20$as$u20$core..iter..traits..iterator..Iterator$GT$::try_fold::enumerate::_$u7b$$u7b$closure$u7d$$u7d$::h4cc1a115f445022d /rustc/2f391da2e6ac73faa3570b79de239fd8c0edf1a9/library/core/src/iter/adapters/enumerate.rs:83:27
    #4 0x7fffee5fa4a0 in core::iter::traits::iterator::Iterator::try_fold::h7899f23c5daa7a1a /rustc/2f391da2e6ac73faa3570b79de239fd8c0edf1a9/library/core/src/iter/traits/iterator.rs:1997:21
    #5 0x7fffee5fa4a0 in _$LT$core..iter..adapters..enumerate..Enumerate$LT$I$GT$$u20$as$u20$core..iter..traits..iterator..Iterator$GT$::try_fold::h4988db12392929eb /rustc/2f391da2e6ac73faa3570b79de239fd8c0edf1a9/library/core/src/iter/adapters/enumerate.rs:89:9
    #6 0x7fffee5fa4a0 in core::iter::traits::iterator::Iterator::find_map::hf9adc9fdda21b25e /rustc/2f391da2e6ac73faa3570b79de239fd8c0edf1a9/library/core/src/iter/traits/iterator.rs:2416:9
    #7 0x7fffee5fa4a0 in _$LT$core..iter..adapters..filter_map..FilterMap$LT$I$C$F$GT$$u20$as$u20$core..iter..traits..iterator..Iterator$GT$::next::hd1303544bd8fcb56 /rustc/2f391da2e6ac73faa3570b79de239fd8c0edf1a9/library/core/src/iter/adapters/filter_map.rs:61:9
    #8 0x7fffee5fa4a0 in wgpu_core::device::_$LT$impl$u20$wgpu_core..hub..Global$LT$G$GT$$GT$::poll_devices::h1cd69ea247a1b141 gfx/wgpu/wgpu-core/src/device/mod.rs:4570:28
    #9 0x7fffee5fa4a0 in wgpu_core::device::_$LT$impl$u20$wgpu_core..hub..Global$LT$G$GT$$GT$::poll_all_devices::hd0c62dbd5428809d gfx/wgpu/wgpu-core/src/device/mod.rs:4583:13
    #10 0x7fffee48870c in wgpu_server_poll_all_devices gfx/wgpu_bindings/src/server.rs:84:5
    #11 0x7fffe3547e58 in mozilla::webgpu::WebGPUParent::RecvShutdown() dom/webgpu/ipc/WebGPUParent.cpp:738:3
    #12 0x7fffdde815f5 in mozilla::webgpu::PWebGPUParent::OnMessageReceived(IPC::Message const&) objdir-ff-asan/ipc/ipdl/PWebGPUParent.cpp:1788:56
    #13 0x7fffdcd6909d in mozilla::layers::PCompositorManagerParent::OnMessageReceived(IPC::Message const&) objdir-ff-asan/ipc/ipdl/PCompositorManagerParent.cpp:200:32
    #14 0x7fffdc86dfac in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) ipc/glue/MessageChannel.cpp:2099:25
    #15 0x7fffdc869eed in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) ipc/glue/MessageChannel.cpp:2023:9
    #16 0x7fffdc86c409 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) ipc/glue/MessageChannel.cpp:1861:3
    #17 0x7fffdc86ced0 in mozilla::ipc::MessageChannel::MessageTask::Run() ipc/glue/MessageChannel.cpp:1899:13
    #18 0x7fffdb0b9e8a in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1142:16
    [...]

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/home/user/firefox/libxul.so+0x1a53d4a0)
Thread T24 (Compositor) created by T0 here:
    #0 0x555555661fec in pthread_create _asan_rtl_:3
    #1 0x7ffff3fd8d73 in _PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:458:14
    #2 0x7ffff3fc264e in PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:533:12
    #3 0x7fffdb0b4f77 in nsThread::Init(nsTSubstring<char> const&) xpcom/threads/nsThread.cpp:602:18
    #4 0x7fffdb0c3eb9 in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, unsigned int, nsIThread**) xpcom/threads/nsThreadManager.cpp:574:12
    #5 0x7fffdb0d104f in NS_NewNamedThread(nsTSubstring<char> const&, nsIThread**, already_AddRefed<nsIRunnable>, unsigned int) xpcom/threads/nsThreadUtils.cpp:162:57
    #6 0x7fffdf66e263 in nsresult NS_NewNamedThread<11ul>(char const (&) [11ul], nsIThread**, already_AddRefed<nsIRunnable>, unsigned int) objdir-ff-asan/dist/include/nsThreadUtils.h:74:10
    #7 0x7fffdf66e263 in mozilla::layers::CompositorThreadHolder::CreateCompositorThread() gfx/layers/ipc/CompositorThread.cpp:62:17
    #8 0x7fffdf66e692 in mozilla::layers::CompositorThreadHolder::CompositorThreadHolder() gfx/layers/ipc/CompositorThread.cpp:39:25
    #9 0x7fffdf66e692 in mozilla::layers::CompositorThreadHolder::Start() gfx/layers/ipc/CompositorThread.cpp:103:33
    #10 0x7fffdf712e27 in gfxPlatform::Init() gfx/thebes/gfxPlatform.cpp:971:3
    [...]

I have a local testcase for this issue which requires a specially patched build and an LD_PRELOAD shim to reproduce the crash, so I can validate potential fixes but for now it is hard to provide the testcase via Bugzilla (in the future, this process will be improved).

I grabbed a core dump for this issue and looked what exactly is causing the crash:

(gdb) f 12
#12 wgpu_core::hub::Storage<T,I>::iter:: () at gfx/wgpu/wgpu-core/src/hub.rs:220
220                     Element::Occupied(ref value, storage_epoch) => {
(gdb) x /i $pc
=> 0x7fd055b7e4a0 <wgpu_core::device::<impl wgpu_core::hub::Global<G>>::poll_all_devices+224>:  cmpl   $0x1,(%r14)
(gdb) info reg r14
r14            0xe5e5e5e5e5e5e5e5       -1880844493789993499

This register pattern is used in our ASan configuration to indicate free'd memory. I don't know why this wouldn't trigger the usual use-after-free report. The fuzzer is sending IPC messages from the child to parent process (potentially multiple messages). If necessary, I can try to extract the series of messages, but maybe the bug already becomes apparent just by checking what happens if you call WebGPUParent::RecvShutdown in abitrary state or maybe twice.

Marking s-s due to potential use-after-free. Should this bug turn out to not be exploitable, please leave it locked to not draw attention to our IPC fuzzing experiments.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Andrew McCreight [:mccr8]]

dom.webgpu.enabled is false by default. Did you enable it when doing this fuzzing?

Comment 3

[Nika Layzell [:nika] (ni? for response)]

(In reply to Andrew McCreight [:mccr8] from comment #2)

dom.webgpu.enabled is false by default. Did you enable it when doing this fuzzing?

Based on https://searchfox.org/mozilla-central/rev/d3683dbb252506400c71256ef3994cdbdfb71ada/gfx/layers/ipc/CompositorBridgeParent.cpp#1864-1869, it appears as though the IPC layer performs no checks to make sure that WebGPU is enabled, and you can create a PWebGPU from a content process even if it is disabled, which is probably what is happening here.

When fuzzing, if the feature is disabled only on the calling side, it's still potentially a callable API on the parent process side.

Comment 4

[Andrew McCreight [:mccr8]]

Ah, great. I was wondering if it might be something like that, which is why I was asking. But I should have looked at the code. Thanks for checking.

It seems bad that we can create these protocols and we don't do any checking, given that WebGPU sec issues are currently being ignored because it is disabled. Though most of those are content process issues, IIRC.

Comment 5

[Daniel Veditz [:dveditz]]

Could this happen with other similar features that are disabled by default?

Comment 7

[Dzmitry Malyshau [:kvark]]

Going to gate the IPC init from the GPU process side, which would make us close this security hole. And then I can look at an actual problem.

Comment 8

[Dzmitry Malyshau [:kvark]]

Attachment: Check for the WebGPU pref before initializing the GPU process IPC side

Comment 9

[Dzmitry Malyshau [:kvark]]

Comment on attachment 9240422 [details]
Check for the WebGPU pref before initializing the GPU process IPC side

Security Approval Request

• How easily could an exploit be constructed based on the patch?: not easily
• 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?: fairly straightforward
• How likely is this patch to cause regressions; how much testing does it need?: unlikely

Comment 10

[Daniel Veditz [:dveditz]]

Which older supported branches are affected by this flaw?: all

When you say "all" does that include ESR-78?

Comment 11

[Daniel Veditz [:dveditz]]

The pref exists so I'm going to assume "yes" and mark the status-firefox fields accordingly

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 9240422 [details]
Check for the WebGPU pref before initializing the GPU process IPC side

sec-approval = dveditz

Comment 13

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

Check for the WebGPU pref before initializing the GPU process IPC side r=aosmond
https://hg.mozilla.org/integration/autoland/rev/33918b139eff008a9cb99b0b4fece919e5e556c8
https://hg.mozilla.org/mozilla-central/rev/33918b139eff

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 15

[Dzmitry Malyshau [:kvark]]

Filled out

Comment 16

[Ryan VanderMeulen [:RyanVM]]

This grafts cleanly to Beta as-landed, but needs rebased patches for ESR91 and ESR78. Please attach those and nominate for approval when you get a chance.

Comment 17

[Dzmitry Malyshau [:kvark]]

Attachment: Check-for-the-WebGPU-pref-ESR78.patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: potential attack vector
Fix Landed on Version: https://hg.mozilla.org/mozilla-central/rev/33918b139eff
Risk to taking this patch (and alternatives if risky): low

Comment 18

[Dzmitry Malyshau [:kvark]]

Attachment: Check-for-the-WebGPU-pref-ESR91.patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: possible attack vector
Fix Landed on Version: https://hg.mozilla.org/mozilla-central/rev/33918b139eff
Risk to taking this patch (and alternatives if risky): low

Comment 19

[Dzmitry Malyshau [:kvark]]

Patches are here, but I haven't confirmed that they build properly. Try pushes are sad:

BugbugTimeoutException: Timed out waiting for result from 'https://bugbug.herokuapp.com/push/try/d8fe879b99a0151bba2d6206af0791ef5892e17c/schedules'

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9240422 [details]
Check for the WebGPU pref before initializing the GPU process IPC side

Looks like the Beta approval request was missed.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9241650 [details] [diff] [review]
Check-for-the-WebGPU-pref-ESR91.patch

Approved for 91.2esr and 78.15esr.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/989bde68af09
https://hg.mozilla.org/releases/mozilla-esr78/rev/e1d4e7ed74be

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9240422 [details]
Check for the WebGPU pref before initializing the GPU process IPC side

Approved for 93.0b7.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a6e059349151

Comment 25

[Tom Ritter [:tjr]]

Did the underlying issue get a bug filed for it?

Comment 26

[Dzmitry Malyshau [:kvark]]

Created bug 1743204

Comment 27

[Dzmitry Malyshau [:kvark]]

Christian, could you share more information about the original issue, if you still have it?
The crash dump, repro instructions, or a Pernosco session, would be helpful.

Comment 28

[Christian Holler (:decoder)]

This ran in a custom IPC fuzzing build and we can't provide a Pernosco session for it unfortunately.

We might be able to provide repro instructions for the revision in comment 0, but it would also require an extensive patch and a binary that needs to be preloaded on Linux. Would that help?

Other than that, we only have the attached crash information and what is in comment 0. The issue definitely looks like a use-after-free when WebGPU is shut down and maybe it would be easier to identify and fix this if we had a proper ASan use-after-free report (it is unclear to me why we don't have it).

We might also be able to find this again once we have implemented the IPC fuzzing target in-tree and temporarily re-enable WebGPU via IPC for testing.

Comment 29

[Dzmitry Malyshau [:kvark]]

Patching and hooking up a binary sounds fine to me. It's not like I have to produce a patch, anyway, I just need to apply it :)