GNOME Bugzilla – Bug 785320
heap-user-after-free when show-all-sources is enabled
Last modified: 2018-06-01 15:07:46 UTC
Forwarding from https://bugs.debian.org/869391 ---------- I enabled all input sources in the tweak tool. Now I can't add input sources. g-c-c crashes with the following backtrace below. Inspecting with gdb suggests that the GList returned by gnome_xkb_info_get_layouts_for_language is invalid, as the data pointed by one of the entries is not readable: (gdb) p (char*)list->data $2 = 0x938 <error: Cannot access memory at address 0x938>
+ Trace 237690
---------- I've confirmed this with version 3.22.2-1 of libgnome-desktop-3-12 on Debian and with gnome-desktop's git master. Though master doesn't crash, running with ASAN still reports a heap-use-after-free error: ==5423==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200032d570 at pc 0x7efbf6e85bb0 bp 0x7ffbffff94b0 sp 0x7ffbffff8c60 READ of size 1 at 0x60200032d570 thread T0 #0 0x7efbf6e85baf (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x44baf) #1 0x7efbef858d58 in g_str_equal /home/jason/devel/checkout/glib/glib/ghash.c:1848 #2 0x7efbef857139 in g_hash_table_lookup_node /home/jason/devel/checkout/glib/glib/ghash.c:396 #3 0x7efbef8583e5 in g_hash_table_contains /home/jason/devel/checkout/glib/glib/ghash.c:1327 #4 0x7efbf3942eef in add_layout_to_table /home/jason/devel/checkout/gnome-desktop/libgnome-desktop/gnome-xkb-info.c:301 #5 0x7efbf3942fba in add_layout_to_locale_tables /home/jason/devel/checkout/gnome-desktop/libgnome-desktop/gnome-xkb-info.c:331 #6 0x7efbf3943313 in parse_end_element /home/jason/devel/checkout/gnome-desktop/libgnome-desktop/gnome-xkb-info.c:411 #7 0x7efbef870ade in emit_end_element /home/jason/devel/checkout/glib/glib/gmarkup.c:1076 #8 0x7efbef871dca in g_markup_parse_context_parse /home/jason/devel/checkout/glib/glib/gmarkup.c:1618 #9 0x7efbf394376e in parse_rules_file /home/jason/devel/checkout/gnome-desktop/libgnome-desktop/gnome-xkb-info.c:535 ... The problem is in the parse_end_element function of gnome-xkb-info.c when it encounters a duplicate layout. It will add the first item to a hash table. Later when processing the duplicate it will free the memory from the first item while it still exists in the hash table.
Created attachment 356241 [details] [review] Fix-heap-use-after-free-with-duplicate-xkb-layouts.patch Attached patch fixes this crash. It checks if the layout is already in the table before adding it.
Review of attachment 356241 [details] [review]: ::: libgnome-desktop/gnome-xkb-info.c @@ +384,3 @@ } + if (g_hash_table_contains (priv->layouts_table, priv->current_parser_layout->id)) { This is already there, see lines 380-383 just above @@ +411,3 @@ + if (g_hash_table_contains (priv->layouts_table, priv->current_parser_variant->id)) { + free_layout (priv->current_parser_variant); This is the one that's missing, I didn't consider this possibility when I wrote https://git.gnome.org/browse/gnome-desktop/commit/?id=b3e4aef Can you please change this patch to just add a similar early return here as the one I pointed to above?
Created attachment 356333 [details] [review] Fix-heap-use-after-free-with-duplicate-xkb-layouts.patch (In reply to Rui Matos from comment #2) > This is already there, see lines 380-383 just above Sorry I hadn't noticed this. I've copied and adapted this for the variant section.
Review of attachment 356333 [details] [review]: looks good, thanks
cf1b213..6c6cc27 gnome-3-24 -> gnome-3-24 a3c7ec8..dda6759 master -> master