From b5fdcfcb5b31a639d82cc79920ecf830c33ec450 Mon Sep 17 00:00:00 2001
From: John Ralls <jralls@ceridwen.us>
Date: Fri, 3 Jan 2020 13:13:55 -0800
Subject: [PATCH] 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.
---
 .../register-gnome/gnucash-item-edit.c        | 40 ++++++++++++++++++-
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/gnucash/register/register-gnome/gnucash-item-edit.c b/gnucash/register/register-gnome/gnucash-item-edit.c
index d9b79e54d3..bce88ead55 100644
--- a/gnucash/register/register-gnome/gnucash-item-edit.c
+++ b/gnucash/register/register-gnome/gnucash-item-edit.c
@@ -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)
 {