segfault (object partially on an unallocated page — possible UaF?) in ElectronDesktopWindowTreeHostLinux::OnWindowTiledStateChanged(ui::WindowTiledEdges)

[brjsp]

We've been getting intermittent crashes in a custom build of Electron 29.3.1 on openSUSE Tumbleweed x64 with the following stack trace:

#0  0x00005583d8b3e4f7 in non-virtual thunk to electron::ElectronDesktopWindowTreeHostLinux::OnWindowTiledStateChanged(ui::WindowTiledEdges) () at ../../electron/shell/browser/ui/electron_desktop_window_tree_host_linux.h:50
#1  0x00005583d913a0dd in ui::X11Window::OnXWindowStateChanged (this=0x3b88001032c0) at ../../base/allocator/partition_allocator/src/partition_alloc/pointers/raw_ptr.h:925
#2  ui::X11Window::UpdateWindowProperties (this=0x3b88001032c0, new_window_properties=...) at ../../ui/ozone/platform/x11/x11_window.cc:2538
#3  0x00005583d913c073 in ui::X11Window::UpdateWindowProperties (new_window_properties=..., this=0x3b88001032c0) at ../../ui/ozone/platform/x11/x11_window.cc:2525
#4  ui::X11Window::OnWMStateUpdated (this=0x3b88001032c0) at ../../ui/ozone/platform/x11/x11_window.cc:2508
#5  ui::X11Window::HandleEvent (this=0x3b88001032c0, xev=...) at ../../ui/ozone/platform/x11/x11_window.cc:2377
#6  0x00005583dbb6f420 in x11::Connection::DispatchEvent (this=this@entry=0x3b88002bd000, event=...) at ../../ui/gfx/x/connection.cc:541
#7  0x00005583dbb6f93b in x11::Connection::ProcessNextEvent (this=this@entry=0x3b88002bd000) at ../../ui/gfx/x/connection.cc:640
#8  0x00005583dbb6fa21 in x11::Connection::Dispatch (this=0x3b88002bd000) at ../../ui/gfx/x/connection.cc:513
#9  0x00005583dbbf2d1d in ui::(anonymous namespace)::XSourceDispatch (source=<optimized out>, unused_func=<optimized out>, data=<optimized out>) at ../../ui/events/platform/x11/x11_event_watcher_glib.cc:57
#10 0x00007f4bb5b77710 in g_main_dispatch (context=0x15b8002e1080) at ../glib/gmain.c:3344
#11 g_main_context_dispatch_unlocked (context=context@entry=0x15b8002e1080) at ../glib/gmain.c:4152

Let's disassemble the offending function:

0000000000d234d0 <non-virtual thunk to electron::ElectronDesktopWindowTreeHostLinux::OnWindowTiledStateChanged(ui::WindowTiledEdges)>:
non-virtual thunk to electron::ElectronDesktopWindowTreeHostLinux::OnWindowTiledStateChanged(ui::WindowTiledEdges):
  d234d0:       53                      push   rbx
  d234d1:       48 8b 87 80 01 00 00    mov    rax,QWORD PTR [rdi+0x180]
  d234d8:       48 89 fb                mov    rbx,rdi
  d234db:       48 8d 3d fe 75 0d 09    lea    rdi,[rip+0x90d75fe]        # 9dfaae0 <features::kWaylandWindowDecorations[abi:logically_const]>
  d234e2:       48 8b 80 50 02 00 00    mov    rax,QWORD PTR [rax+0x250]
  d234e9:       48 8b 80 60 01 00 00    mov    rax,QWORD PTR [rax+0x160]
  d234f0:       48 8b 80 38 03 00 00    mov    rax,QWORD PTR [rax+0x338]
  d234f7:       89 b0 b8 04 00 00       mov    DWORD PTR [rax+0x4b8],esi
  d234fd:       e8 be df 5a 02          call   32d14c0 <base::FeatureList::IsEnabled(base::Feature[abi:logically_const] const&)>
  d23502:       84 c0                   test   al,al
  d23504:   /-- 75 0a                   jne    d23510 <non-virtual thunk to electron::ElectronDesktopWindowTreeHostLinux::OnWindowTiledStateChanged(ui::WindowTiledEdges)+0x40>
  d23506:   |   5b                      pop    rbx
  d23507:   |   c3                      ret
  d23508:   |   0f 1f 84 00 00 00 00    nop    DWORD PTR [rax+rax*1+0x0]
  d2350f:   |   00 
  d23510:   \-> 48 8d bb a0 fe ff ff    lea    rdi,[rbx-0x160]
  d23517:       5b                      pop    rbx
  d23518:       e9 13 fc ff ff          jmp    d23130 <electron::ElectronDesktopWindowTreeHostLinux::UpdateFrameHints() [clone .part.0]>
  d2351d:       90                      nop
  d2351e:       66 90                   xchg   ax,ax

The faulting instruction is the write at 0xd234f7. If we compare that with the source code:
https://github.com/electron/electron/blob/v29.3.1/shell/browser/ui/electron_desktop_window_tree_host_linux.cc#L72

void ElectronDesktopWindowTreeHostLinux::OnWindowTiledStateChanged(
    ui::WindowTiledEdges new_tiled_edges) {
  static_cast<ClientFrameViewLinux*>(
      native_window_view_->widget()->non_client_view()->frame_view())
      ->set_tiled_edges(new_tiled_edges);
  UpdateFrameHints();
}

https://github.com/electron/electron/blob/v29.3.1/shell/browser/ui/views/client_frame_view_linux.h#L49

class ClientFrameViewLinux : public FramelessView,
                             public ui::NativeThemeObserver,
                             public ui::WindowButtonOrderObserver {
//…
  void set_tiled_edges(ui::WindowTiledEdges tiled_edges) {
    tiled_edges_ = tiled_edges;
  }

…it is clear that it's this assignment.
The address at rax is the views::NonClientFrameView base of the electron::ClientFrameViewLinux, and rsi are the flags we are trying to write:

(gdb) info registers
rax            0x3b88001dac80      65455303535744
rsi            0x1010101           16843009

There should be no offset between ClientFrameViewLinux and its NonClientFrameView base (if i'm interpreting the (gdb) print *(electron::ClientFrameViewLinux*)0xsome_address output correctly).

Let's look at the memory mappings (I only have access to core dumps because the crashes are intermittent):

(gdb) maintanance info sections
[287]      0x3b8800174000->0x3b88001db000 at 0x0479d000: load144 ALLOC LOAD HAS_CONTENTS
[288]      0x3b88001db000->0x3b88001dc000 at 0x04804000: load145 ALLOC READONLY

How large is this object?

(gdb) p/x sizeof(electron::ClientFrameViewLinux)
$7 = 0x4c0

The object overhangs the READONLY page by 0x140 bytes. That page seems to be all zeroes.
At this point i'm unsure what to do next — it'd better not be an allocator bug…

[electron-issue-triage]

Hello @brjsp. Thanks for reporting this and helping to make Electron better!

Would it be possible for you to make a standalone testcase with only the code necessary to reproduce the issue? For example, Electron Fiddle is a great tool for making small test cases and makes it easy to publish your test case to a gist that Electron maintainers can use.

Stand-alone test cases make fixing issues go more smoothly: it ensure everyone's looking at the same issue, it removes all unnecessary variables from the equation, and it can also provide the basis for automated regression tests.

Now adding the blocked/need-repro Needs a test case to reproduce the bug label for this reason. After you make a test case, please link to it in a followup comment. This issue will be closed in 10 days if the above is not addressed.

[VerteDinde]

Hey @brjsp, I appreciate the write up here! We have a similar crash reported on various Linux distros elsewhere in the issue tracker, but the maintainers have unfortunately been unable to reproduce the crash on any Ubuntu devices - if you happen to have an application or gist repro that we could try to see if we can reproduce the crash, that would be extremely helpful. I'll dig into your writeup here in the meantime.

[brjsp]

@VerteDinde I don't even know if this can be reproduced with official builds.

The crashing application is signal-desktop. For me, the crashes happen intermittently, but @JohnVeness has reported they can reproduce it every time.

@JohnVeness Can you download the electron 29.3.1 binary from github and try to run Signal with it? You can do that by adding the folder with the unpacked electron binary before /usr/bin in
PATH and starting signal-desktop from the commandline.

[JohnVeness]

@JohnVeness Can you download the electron 29.3.1 binary from github and try to run Signal with it? You can do that by adding the folder with the unpacked electron binary before /usr/bin in PATH and starting signal-desktop from the commandline.

Hi. I have just tried and the segfault does not occur with the official 29.3.1 binary (it still occurs every time without that binary).

[brjsp]

Building with use_allocator_shim=false enable_backup_ref_ptr_support=false seems to make the crashes go away.