[geany/geany-plugins] codenav/goto_file: Fix use-after-free crash (PR #1339) - Github-comments

25 Apr 2024


      Hi, this PR fixes a use-after-free in codenav&#39;s go-to-file. (which often results in a crash)
## Problem:
### How to reproduce
1. Run geany from command line, to observe error messages
2. Enable codenav plugin, open some document (to allow use of go-to tool)
3. Trigger go-to-file tool (e.g. via shortcut)
4. Write `/`
5. Cancel &amp; close dialog
6. Trigger go-to-file again
7. Write `a`
-&gt; You will likely see an assertion fail in the output, or if you are lucky geany will crash altogether:
`gtk_entry_completion_set_model: assertion &#39;model == NULL || GTK_IS_TREE_MODEL (model)&#39; failed`
Valgrind shows an invalid read to previously freed memory:
&lt;details&gt;
```
Invalid read of size 8
  at 0x4DD5820: UnknownInlinedFun (gtkentrycompletion.c:1224)
  by 0x4DD5820: gtk_entry_completion_set_model (gtkentrycompletion.c:1220)
  by 0xE2E89F3: directory_check (goto_file.c:166)
  by 0x5A6B72F: g_closure_invoke (gclosure.c:834)
  by 0x5A9AC1A: signal_emit_unlocked_R.isra.0 (gsignal.c:3961)
  by 0x5A8B7A1: signal_emit_valist_unlocked (gsignal.c:3520)
  by 0x5A8BCAF: g_signal_emit_by_name (gsignal.c:3624)
  by 0x4DBAC29: end_change.lto_priv.0 (gtkentry.c:2941)
  by 0x4DC6576: gtk_entry_real_insert_text.lto_priv.0 (gtkentry.c:5401)
  by 0x5A6B72F: g_closure_invoke (gclosure.c:834)
  by 0x5A9AF49: signal_emit_unlocked_R.isra.0 (gsignal.c:3928)
  by 0x5A8B7A1: signal_emit_valist_unlocked (gsignal.c:3520)
  by 0x5A8BCAF: g_signal_emit_by_name (gsignal.c:3624)
Address 0xd42b8e0 is 96 bytes inside a block of size 136 free&#39;d
  at 0x48468CF: free (vg_replace_malloc.c:985)
  by 0x5A90164: g_type_free_instance (gtype.c:2023)
  by 0x5A7A732: g_object_unref (gobject.c:4475)
  by 0x48E5AC9: run_kb (keybindings.c:1334)
  by 0x48E5AC9: run_kb (keybindings.c:1325)
  by 0x48E67E4: on_key_press_event (keybindings.c:1396)
  by 0x4CFA6CC: _gtk_marshal_BOOLEAN__BOXED.part.0 (gtkmarshalers.c:84)
  by 0x5A6B72F: g_closure_invoke (gclosure.c:834)
  by 0x5A9A895: signal_emit_unlocked_R.isra.0 (gsignal.c:3888)
  by 0x5A8B094: signal_emit_valist_unlocked (gsignal.c:3533)
  by 0x5A8B9D6: g_signal_emit_valist (gsignal.c:3263)
  by 0x5A8BA93: g_signal_emit (gsignal.c:3583)
  by 0x4FC2CD4: gtk_widget_event_internal.part.0.lto_priv.0 (gtkwidget.c:7812)
Block was alloc&#39;d at
  at 0x484A993: calloc (vg_replace_malloc.c:1595)
  by 0x5B2651A: g_malloc0 (gmem.c:133)
  by 0x5A96F1B: g_type_create_instance (gtype.c:1923)
  by 0x5A7CB10: g_object_new_internal.part.0 (gobject.c:2603)
  by 0x5A7E0C6: UnknownInlinedFun (gobject.c:2600)
  by 0x5A7E0C6: g_object_new_with_properties (gobject.c:2766)
  by 0x5A7F009: g_object_new (gobject.c:2412)
  by 0x4E56A25: gtk_list_store_new (gtkliststore.c:426)
  by 0xE2E8573: build_file_list (goto_file.c:111)
  by 0xE2E86A7: menu_item_activate (goto_file.c:285)
  by 0x48E5AC9: run_kb (keybindings.c:1334)
  by 0x48E5AC9: run_kb (keybindings.c:1325)
  by 0x48E67E4: on_key_press_event (keybindings.c:1396)
  by 0x4CFA6CC: _gtk_marshal_BOOLEAN__BOXED.part.0 (gtkmarshalers.c:84)
Invalid read of size 8
  at 0x5A92C79: g_type_check_instance_is_a (gtype.c:4133)
  by 0x4DD5836: UnknownInlinedFun (gtkentrycompletion.c:1224)
  by 0x4DD5836: gtk_entry_completion_set_model (gtkentrycompletion.c:1220)
  by 0xE2E89F3: directory_check (goto_file.c:166)
  by 0x5A6B72F: g_closure_invoke (gclosure.c:834)
  by 0x5A9AC1A: signal_emit_unlocked_R.isra.0 (gsignal.c:3961)
  by 0x5A8B7A1: signal_emit_valist_unlocked (gsignal.c:3520)
  by 0x5A8BCAF: g_signal_emit_by_name (gsignal.c:3624)
  by 0x4DBAC29: end_change.lto_priv.0 (gtkentry.c:2941)
  by 0x4DC6576: gtk_entry_real_insert_text.lto_priv.0 (gtkentry.c:5401)
  by 0x5A6B72F: g_closure_invoke (gclosure.c:834)
  by 0x5A9AF49: signal_emit_unlocked_R.isra.0 (gsignal.c:3928)
  by 0x5A8B7A1: signal_emit_valist_unlocked (gsignal.c:3520)
Address 0xd42b8e0 is 96 bytes inside a block of size 136 free&#39;d
  at 0x48468CF: free (vg_replace_malloc.c:985)
  by 0x5A90164: g_type_free_instance (gtype.c:2023)
  by 0x5A7A732: g_object_unref (gobject.c:4475)
  by 0x48E5AC9: run_kb (keybindings.c:1334)
  by 0x48E5AC9: run_kb (keybindings.c:1325)
  by 0x48E67E4: on_key_press_event (keybindings.c:1396)
  by 0x4CFA6CC: _gtk_marshal_BOOLEAN__BOXED.part.0 (gtkmarshalers.c:84)
  by 0x5A6B72F: g_closure_invoke (gclosure.c:834)
  by 0x5A9A895: signal_emit_unlocked_R.isra.0 (gsignal.c:3888)
  by 0x5A8B094: signal_emit_valist_unlocked (gsignal.c:3533)
  by 0x5A8B9D6: g_signal_emit_valist (gsignal.c:3263)
  by 0x5A8BA93: g_signal_emit (gsignal.c:3583)
  by 0x4FC2CD4: gtk_widget_event_internal.part.0.lto_priv.0 (gtkwidget.c:7812)
Block was alloc&#39;d at
  at 0x484A993: calloc (vg_replace_malloc.c:1595)
  by 0x5B2651A: g_malloc0 (gmem.c:133)
  by 0x5A96F1B: g_type_create_instance (gtype.c:1923)
  by 0x5A7CB10: g_object_new_internal.part.0 (gobject.c:2603)
  by 0x5A7E0C6: UnknownInlinedFun (gobject.c:2600)
  by 0x5A7E0C6: g_object_new_with_properties (gobject.c:2766)
  by 0x5A7F009: g_object_new (gobject.c:2412)
  by 0x4E56A25: gtk_list_store_new (gtkliststore.c:426)
  by 0xE2E8573: build_file_list (goto_file.c:111)
  by 0xE2E86A7: menu_item_activate (goto_file.c:285)
  by 0x48E5AC9: run_kb (keybindings.c:1334)
  by 0x48E5AC9: run_kb (keybindings.c:1325)
  by 0x48E67E4: on_key_press_event (keybindings.c:1396)
  by 0x4CFA6CC: _gtk_marshal_BOOLEAN__BOXED.part.0 (gtkmarshalers.c:84)
Gtk-CRITICAL **: 22:43:16.822: gtk_entry_completion_set_model: assertion &#39;model == NULL || GTK_IS_TREE_MODEL (model)&#39; failed
```
&lt;/details&gt;
## Cause
From what I understand, the root cause is here:
https://github.com/geany/geany-plugins/blob/2b897dc56c097551d2214aef9bfe2c93...
The model is saved in `old_model`, but the refcount is not incremented. As soon as the entry is destroyed, the model&#39;s refcount drops to 0 and it&#39;s deallocated. The (static) `old_model` now points to freed memory. In the next invocation, `gtk_entry_completion_set_model()` will be called with `old_model`, resulting in an invalid read in freed memory.
## Fix
Increment ref count when caching the completion model (what this PR does).
Extra evidence: With this PR applied, add a print for the refcount before gtk_entry_completion_set_model()`, like so:
```c
printf(&quot;old model %p refcount %u\n&quot;, old_model, ((GObject *) old_model)-&gt;ref_count);
gtk_entry_completion_set_model (completion, old_model);
```
This shows a refcount of 1 at that point, so if not for the `g_object_ref()` call added in this PR, this would have been 0, and the model would have been freed.
You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/1339
-- Commit Summary --
* codenav: Fix use-after-free in cached completion model
-- File Changes --
M codenav/src/goto_file.c (6)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/1339.patch
https://github.com/geany/geany-plugins/pull/1339.diff
-- 
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1339
You are receiving this because you are subscribed to this thread.

Message ID: &lt;geany/geany-plugins/pull/1339@github.com&gt;