Bug 797481 - crash on close of unsaved tabs by pressing [X]

Use g_object_weak_ref() to null GncItemEdit::sheet to prevent
a use-after-free crash.

Note that this is a band-aid fix. To fix correctly GncItemEdit and probably
most of the rest of the register must be rewritten with modern GObject
macros and resource management.
This commit is contained in:
John Ralls 2020-01-03 13:13:55 -08:00
parent d409d009fb
commit b5fdcfcb5b

View File

@ -229,6 +229,9 @@ gnc_item_edit_get_pixel_coords (GncItemEdit *item_edit,
SheetBlock *block;
int xd, yd;
if (sheet == NULL)
return;
block = gnucash_sheet_get_block (sheet, item_edit->virt_loc.vcell_loc);
if (block == NULL)
return;
@ -257,6 +260,8 @@ gnc_item_edit_update (GncItemEdit *item_edit)
{
gint x = 0, y = 0, w, h;
if (item_edit == NULL || item_edit->sheet == NULL)
return FALSE;
gnc_item_edit_get_pixel_coords (item_edit, &x, &y, &w, &h);
gtk_layout_move (GTK_LAYOUT(item_edit->sheet),
GTK_WIDGET(item_edit), x, y);
@ -657,6 +662,28 @@ gnc_item_edit_get_property (GObject *object,
}
}
/* FIXME: Idle events calling gnc_item_edit_update can fire after the
* GncItemEdit is finalized, but because the GncItemEdit class is
* defined by hand instead of with the GType macros there's no way to
* run gnc_item_edit_dispose or gnc_item_edit_finalize to null out the
* pointers. We resort instead to a weak reference to null out the
* sheet when it gets finalized to prevent gnc_item_edit_upate from
* accessing the freed sheet.
*
* https://bugs.gnucash.org/show_bug.cgi?id=797481
*
* Note that this is still not bulletproof, after all we're still
* using the GncItemEdit after it has been freed but without a dispose
* function to do this correctly we're a bit stuck.
*/
static void
sheet_destroyed (gpointer data, GObject *table)
{
GncItemEdit *item_edit = (GncItemEdit*)data;
if (item_edit)
item_edit->sheet = NULL;
}
static void
gnc_item_edit_set_property (GObject *object,
guint param_id,
@ -664,11 +691,18 @@ gnc_item_edit_set_property (GObject *object,
GParamSpec *pspec)
{
GncItemEdit *item_edit = GNC_ITEM_EDIT (object);
switch (param_id)
{
case PROP_SHEET:
if (item_edit->sheet)
g_object_weak_unref(G_OBJECT(item_edit->sheet),
(GWeakNotify)sheet_destroyed,
(gpointer)item_edit);
item_edit->sheet = GNUCASH_SHEET (g_value_get_object (value));
if (item_edit->sheet)
g_object_weak_ref(G_OBJECT(item_edit->sheet),
(GWeakNotify)sheet_destroyed,
(gpointer)item_edit);
break;
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, param_id, pspec);
@ -731,7 +765,9 @@ gnc_item_edit_class_init (GncItemEditClass *gnc_item_edit_class)
widget_class->get_preferred_height = gnc_item_edit_get_preferred_height;
}
/* FIXME: This way of initializing GObjects is obsolete. We should be
* using G_DECLARE_FINAL_TYPE instead of rolling _get_type by hand.
*/
GType
gnc_item_edit_get_type (void)
{