1631981 - PHC crash in [@ thaw_updates] (UAF)

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Gabriele Svelto [:gsvelto] (PTO)]

This bug is for crash report bp-336d7719-54a6-4dfc-8829-25cae0200317.

Top 10 frames of crashing thread:

0 libgtk-3.so.0.2404.1 thaw_updates ./debian/build/deb/gtk/../../../../gtk/gtkfilesystemmodel.c:2040
1 libgio-2.0.so.0.5800.3 g_task_return_now ./debian/build/deb/../../../gio/gtask.c:1148
2 libgio-2.0.so.0.5800.3 complete_in_idle_cb ./debian/build/deb/../../../gio/gtask.c:1162
3 libglib-2.0.so.0.5800.3 g_main_context_dispatch ./debian/build/deb/../../../glib/gmain.c:3847
4 libglib-2.0.so.0.5800.3 g_main_context_iterate ./debian/build/deb/../../../glib/gmain.c:3920
5 libglib-2.0.so.0.5800.3 g_main_context_iteration ./debian/build/deb/../../../glib/gmain.c:3981
6 libxul.so nsBaseAppShell::OnProcessNextEvent widget/nsBaseAppShell.cpp:259
7 libxul.so non-virtual thunk to nsBaseAppShell::OnProcessNextEvent widget/nsBaseAppShell.cpp
8 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1124
9 libxul.so <name omitted> xpcom/threads/nsThreadUtils.cpp:481

This is an use-after-free in GTK. While this specific crash is using an older version of GTK I found crashes happening with recent versions too, this one is using 3.24.17 for example:

https://crash-stats.mozilla.org/report/index/274d75d8-3301-4e8c-ac5c-ec4c60200410#tab-details

This must have flown under the radar because it manifests itself as a NULL pointer crash on 64-bit builds, but on 32-bit ones you can see the e5e5e5e5 poison pattern of UAFs. This is an unfortunate effect of bug 1493342, we're not really crashing on NULL, we're probably hitting the poison pattern on 64-bit machines too.

Comment 1

[Gabriele Svelto [:gsvelto] (PTO)]

This is happening off of the main event loop in the main process. It could be nasty. Eric, who do we inform to deal with this kind of issues? Martin do you know who should we contact in the GNOME project to inform them of this issue? This is not something I'm comfortable putting in a public bug tracker.

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

I can confirm that the 64-bit crashes are all accessing the poison pattern by downloading a few of them and dumping them out. rax is set to e5e5e5e5e5e5e5e5 in all of them.

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Adding gnome folks here. Jonas, Robert, what's the best way how to handle it?
Thanks.

Comment 4

[Gabriele Svelto [:gsvelto] (PTO)]

Here's the symbolicated stacks of where the affected object was allocated and then freed:

Free stack:

#0    replace_free(void*)
    in file hg:hg.mozilla.org/releases/mozilla-beta:memory/replace/phc/PHC.cpp:048c7cb557453e3abe49216c52fb868ed9a4819c line 1155
#1    g_type_free_instance
    in file ./debian/build/deb/../../../gobject/gtype.c line 1952
#2    search_start_query
    in file ./debian/build/deb/gtk/../../../../gtk/gtkfilechooserwidget.c line 7276
#3    _g_closure_invoke_va
    in file ./debian/build/deb/../../../gobject/gclosure.c line 879
#4    g_signal_emit_valist
    in file ./debian/build/deb/../../../gobject/gsignal.c line 3140
#5    g_signal_emit
    in file ./debian/build/deb/../../../gobject/gsignal.c line 3449
#6    gtk_search_entry_changed_timeout_cb
    in file ./debian/build/deb/gtk/../../../../gtk/gtksearchentry.c line 296
#7    g_timeout_dispatch
    in file ./debian/build/deb/../../../glib/gmain.c line 4667
#8    g_main_context_dispatch
    in file ./debian/build/deb/../../../glib/gmain.c line 3847
#9    g_main_context_iterate
    in file ./debian/build/deb/../../../glib/gmain.c line 3920
#10    g_main_context_iteration
    in file ./debian/build/deb/../../../glib/gmain.c line 3982
#11    nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool)
    in file hg:hg.mozilla.org/releases/mozilla-beta:widget/nsBaseAppShell.cpp:048c7cb557453e3abe49216c52fb868ed9a4819c line 259
#12    non-virtual thunk to nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool)
    in file hg:hg.mozilla.org/releases/mozilla-beta:widget/nsBaseAppShell.cpp:048c7cb557453e3abe49216c52fb868ed9a4819c line 0
#13    nsThread::ProcessNextEvent(bool, bool*)
    in file hg:hg.mozilla.org/releases/mozilla-beta:xpcom/threads/nsThread.cpp:048c7cb557453e3abe49216c52fb868ed9a4819c line 1127
#14    <name omitted>
    in file hg:hg.mozilla.org/releases/mozilla-beta:xpcom/threads/nsThreadUtils.cpp:048c7cb557453e3abe49216c52fb868ed9a4819c line 481
#15    mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)
    in file hg:hg.mozilla.org/releases/mozilla-beta:ipc/glue/MessagePump.cpp:048c7cb557453e3abe49216c52fb868ed9a4819c line 109

Alloc stack:

#0    MaybePageAlloc(mozilla::Maybe<unsigned long> const&, unsigned long, bool)
    in file hg:hg.mozilla.org/releases/mozilla-beta:memory/replace/phc/PHC.cpp:048c7cb557453e3abe49216c52fb868ed9a4819c line 898
#1    <name omitted>
    in file hg:hg.mozilla.org/releases/mozilla-beta:memory/build/malloc_decls.h:048c7cb557453e3abe49216c52fb868ed9a4819c line 51
#2    g_malloc
    in file ./debian/build/deb/../../../glib/gmem.c line 101
#3    g_slice_alloc
    in file ./debian/build/deb/../../../glib/gslice.c line 1024
#4    g_slice_alloc0
    in file ./debian/build/deb/../../../glib/gslice.c line 1050
#5    g_type_create_instance
    in file ./debian/build/deb/../../../gobject/gtype.c line 1846
#6    g_object_new_internal
    in file ./debian/build/deb/../../../gobject/gobject.c line 1805
#7    g_object_new_with_properties
    in file ./debian/build/deb/../../../gobject/gobject.c line 1973
#8    g_object_new
    in file ./debian/build/deb/../../../gobject/gobject.c line 1645
#9    _gtk_file_system_model_new
    in file ./debian/build/deb/gtk/../../../../gtk/gtkfilesystemmodel.c line 1412
#10    search_start_query
    in file ./debian/build/deb/gtk/../../../../gtk/gtkfilechooserwidget.c line 7276
#11    _g_closure_invoke_va
    in file ./debian/build/deb/../../../gobject/gclosure.c line 879
#12    g_signal_emit_valist
    in file ./debian/build/deb/../../../gobject/gsignal.c line 3140
#13    g_signal_emit
    in file ./debian/build/deb/../../../gobject/gsignal.c line 3449
#14    gtk_search_entry_changed_timeout_cb
    in file ./debian/build/deb/gtk/../../../../gtk/gtksearchentry.c line 296
#15    g_timeout_dispatch
    in file ./debian/build/deb/../../../glib/gmain.c line 4667

Comment 5

[Robert Mader [:rmader]]

If this is a use-after-free in GTK I guess the way forward is to create a confidential issue at https://gitlab.gnome.org/GNOME/gtk/-/issues (there is a checkbox when creating an issue) - sorry, can't help with the code in question :/

Gabriele, could you do that?

Comment 6

[Eric Rahm [:erahm]]

(In reply to Gabriele Svelto [:gsvelto] from comment #1)

This is happening off of the main event loop in the main process. It could be nasty. Eric, who do we inform to deal with this kind of issues? Martin do you know who should we contact in the GNOME project to inform them of this issue? This is not something I'm comfortable putting in a public bug tracker.

It looks like the gnome folks are looped in. Our sec team will triage this issue and taker further action if necessary.

Comment 7

[Robert Mader [:rmader]]

Just got reaffirmed by GTK devs that a confidential issue at https://gitlab.gnome.org/GNOME/gtk/-/issues is the right thing (sorry for the noice).

Comment 8

[Gabriele Svelto [:gsvelto] (PTO)]

Alright, I'll do that. I've pored over the stacks for a bit but I'm still not 100% sure if it's a genuine GTK issue or if it's our fault and we're doing something wrong. There's a lot of comments in the crash reports under this signature and many of them mention opening the file picker and then typing to search for a file. I assume that should almost entirely happen within GTK code but I can't be sure.

Comment 9

[Gabriele Svelto [:gsvelto] (PTO)]

Here's the corresponding issue on the GTK tracker.

Comment 10

[Gabriele Svelto [:gsvelto] (PTO)]

This was fixed upstream: https://gitlab.gnome.org/GNOME/gtk/-/commit/326077d2edd2450879a58e5b31f554b9084a7bdc

Should we wait until the next GTK release before closing the bug?

Comment 11

[Gian-Carlo Pascutto [:gcp]]

Marking as stalled because we ourselves can't move this forward.

Comment 12

[Gabriele Svelto [:gsvelto] (PTO)]

I've checked the remaining crash reports and they're all coming from people running older versions of GTK. I'll mark this FIXED given that the upstream patch shipped and was distributed to all major distros months ago.

Comment 13

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

Since the bug is closed, the stalled keyword is now meaningless.
For more information, please visit auto_nag documentation.