[evolution/gnome-3-28] [composer-autosave] Use-after-free during snapshot save to file ][

[Milan Crha <mcrha src gnome org>]

commit f5f57a0f05b6370a655ef8f38f27db275327387b
Author: Milan Crha <mcrha redhat com>
Date:   Tue May 29 12:52:54 2018 +0200

    [composer-autosave] Use-after-free during snapshot save to file ][
    
    Extend the previous change to not do autosave when the composer is busy,
    to avoid calls interleave. When the message is building, the composer
    asks the HTML editor for the message content. This is done with a D-Bus
    call to the WebKitWebProcess, which cannot block the main loop, because
    the WebKit can do its own IPC calls there, thus the GUI would just block
    in that case. Having the GUI updated during the wait for the message
    content can also mean that the user will send the message in that time,
    or the autosave will be triggered, where both can mean a use-after-free
    of the variables being involved in the message building.
    
    This had been reported downstream at:
    https://bugzilla.redhat.com/show_bug.cgi?id=1538865

 src/composer/e-composer-private.c                  |   5 +-
 src/composer/e-composer-private.h                  |   3 +-
 src/composer/e-msg-composer.c                      | 100 +++++++++++++++++----
 src/composer/e-msg-composer.h                      |   1 +
 .../composer-autosave/e-composer-autosave.c        |   8 +-
 5 files changed, 94 insertions(+), 23 deletions(-)
---
diff --git a/src/composer/e-composer-private.c b/src/composer/e-composer-private.c
index fa324f8d4b..5475ae7c68 100644
--- a/src/composer/e-composer-private.c
+++ b/src/composer/e-composer-private.c
@@ -130,7 +130,8 @@ e_composer_private_constructed (EMsgComposer *composer)
 
        priv->set_signature_from_message = FALSE;
        priv->disable_signature = FALSE;
-       priv->busy = FALSE;
+       priv->soft_busy_count = 0;
+       priv->had_activities = FALSE;
        priv->saved_editable = FALSE;
        priv->dnd_history_saved = FALSE;
        priv->check_if_signature_is_changed = FALSE;
@@ -357,7 +358,7 @@ e_composer_private_constructed (EMsgComposer *composer)
         * a simple inverted binding to EMsgComposer's "busy" property. */
 
        e_binding_bind_property (
-               composer, "busy",
+               composer, "soft-busy",
                priv->async_actions, "sensitive",
                G_BINDING_SYNC_CREATE |
                G_BINDING_INVERT_BOOLEAN);
diff --git a/src/composer/e-composer-private.h b/src/composer/e-composer-private.h
index c590aaefd1..9ceb900e22 100644
--- a/src/composer/e-composer-private.h
+++ b/src/composer/e-composer-private.h
@@ -92,7 +92,8 @@ struct _EMsgComposerPrivate {
 
        CamelMimeMessage *redirect;
 
-       gboolean busy;
+       guint soft_busy_count; /* Only to disable async operations when >0 */
+       gboolean had_activities; /* Hard busy flag */
        gboolean disable_signature;
        gboolean is_reply_or_forward;
        /* The web view is uneditable while the editor is busy.
diff --git a/src/composer/e-msg-composer.c b/src/composer/e-msg-composer.c
index e60fae24e5..23c2f0db1a 100644
--- a/src/composer/e-msg-composer.c
+++ b/src/composer/e-msg-composer.c
@@ -90,6 +90,7 @@ typedef enum {
 enum {
        PROP_0,
        PROP_BUSY,
+       PROP_SOFT_BUSY,
        PROP_EDITOR,
        PROP_FOCUS_TRACKER,
        PROP_SHELL,
@@ -169,6 +170,30 @@ async_context_free (AsyncContext *context)
        g_slice_free (AsyncContext, context);
 }
 
+static void
+e_msg_composer_inc_soft_busy (EMsgComposer *composer)
+{
+       g_return_if_fail (E_IS_MSG_COMPOSER (composer));
+       g_return_if_fail (composer->priv->soft_busy_count + 1 > composer->priv->soft_busy_count);
+
+       composer->priv->soft_busy_count++;
+
+       if (composer->priv->soft_busy_count == 1)
+               g_object_notify (G_OBJECT (composer), "soft-busy");
+}
+
+static void
+e_msg_composer_dec_soft_busy (EMsgComposer *composer)
+{
+       g_return_if_fail (E_IS_MSG_COMPOSER (composer));
+       g_return_if_fail (composer->priv->soft_busy_count > 0);
+
+       composer->priv->soft_busy_count--;
+
+       if (composer->priv->soft_busy_count == 0)
+               g_object_notify (G_OBJECT (composer), "soft-busy");
+}
+
 /**
  * emcu_part_to_html:
  * @part:
@@ -1102,10 +1127,10 @@ composer_build_message (EMsgComposer *composer,
        const gchar *from_domain;
        gint i;
 
+       e_msg_composer_inc_soft_busy (composer);
+
        priv = composer->priv;
        table = e_msg_composer_get_header_table (composer);
-       view = e_msg_composer_get_attachment_view (composer);
-       store = e_attachment_view_get_store (view);
 
        identity_uid = e_composer_header_table_dup_identity_uid (table, NULL, NULL);
        if (identity_uid) {
@@ -1154,6 +1179,8 @@ composer_build_message (EMsgComposer *composer,
 
        /* If this is a redirected message, just tweak the headers. */
        if (priv->redirect) {
+               e_msg_composer_dec_soft_busy (composer);
+
                context->skip_content = TRUE;
                context->message = g_object_ref (priv->redirect);
                build_message_headers (composer, context->message, TRUE);
@@ -1474,6 +1501,9 @@ composer_build_message (EMsgComposer *composer,
                g_slist_free_full (inline_images_parts, g_object_unref);
        }
 
+       view = e_msg_composer_get_attachment_view (composer);
+       store = e_attachment_view_get_store (view);
+
        /* If there are attachments, wrap what we've built so far
         * along with the attachments in a multipart/mixed part. */
        if (e_attachment_store_get_num_attachments (store) > 0) {
@@ -1508,6 +1538,8 @@ composer_build_message (EMsgComposer *composer,
        else
                g_simple_async_result_complete (simple);
 
+       e_msg_composer_dec_soft_busy (composer);
+
        g_object_unref (simple);
 }
 
@@ -2163,6 +2195,12 @@ msg_composer_get_property (GObject *object,
                                E_MSG_COMPOSER (object)));
                        return;
 
+               case PROP_SOFT_BUSY:
+                       g_value_set_boolean (
+                               value, e_msg_composer_is_soft_busy (
+                               E_MSG_COMPOSER (object)));
+                       return;
+
                case PROP_EDITOR:
                        g_value_set_object (
                                value, e_msg_composer_get_editor (
@@ -2240,35 +2278,31 @@ composer_notify_activity_cb (EActivityBar *activity_bar,
 {
        EHTMLEditor *editor;
        EContentEditor *cnt_editor;
-       gboolean editable = TRUE;
-       gboolean busy;
+       gboolean has_activities;
 
-       busy = (e_activity_bar_get_activity (activity_bar) != NULL);
+       has_activities = (e_activity_bar_get_activity (activity_bar) != NULL);
 
-       if (busy == composer->priv->busy)
+       if (has_activities == composer->priv->had_activities)
                return;
 
-       composer->priv->busy = busy;
-
-       if (busy)
-               e_msg_composer_save_focused_widget (composer);
+       composer->priv->had_activities = has_activities;
 
        editor = e_msg_composer_get_editor (composer);
        cnt_editor = e_html_editor_get_content_editor (editor);
 
-       if (busy) {
-               editable = e_content_editor_is_editable (cnt_editor);
+       if (has_activities) {
+               e_msg_composer_save_focused_widget (composer);
+
+               composer->priv->saved_editable = e_content_editor_is_editable (cnt_editor);
                e_content_editor_set_editable (cnt_editor, FALSE);
-               composer->priv->saved_editable = editable;
        } else {
-               editable = composer->priv->saved_editable;
-               e_content_editor_set_editable (cnt_editor, editable);
+               e_content_editor_set_editable (cnt_editor, composer->priv->saved_editable);
+
+               e_msg_composer_restore_focus_on_composer (composer);
        }
 
        g_object_notify (G_OBJECT (composer), "busy");
-
-       if (!busy)
-               e_msg_composer_restore_focus_on_composer (composer);
+       g_object_notify (G_OBJECT (composer), "soft-busy");
 }
 
 static void
@@ -2627,7 +2661,24 @@ e_msg_composer_is_busy (EMsgComposer *composer)
 {
        g_return_val_if_fail (E_IS_MSG_COMPOSER (composer), FALSE);
 
-       return composer->priv->busy;
+       return composer->priv->had_activities;
+}
+
+/**
+ * e_msg_composer_is_soft_busy:
+ * @composer: an #EMsgComposer
+ *
+ * Returns: %TRUE when e_msg_composer_is_busy() returns %TRUE or
+ *    when the asynchronous operations are disabled.
+ *
+ * Since: 3.29.3
+ **/
+gboolean
+e_msg_composer_is_soft_busy (EMsgComposer *composer)
+{
+       g_return_val_if_fail (E_IS_MSG_COMPOSER (composer), FALSE);
+
+       return composer->priv->soft_busy_count > 0 || e_msg_composer_is_busy (composer);
 }
 
 static void
@@ -2662,6 +2713,17 @@ e_msg_composer_class_init (EMsgComposerClass *class)
                        G_PARAM_READABLE |
                        G_PARAM_STATIC_STRINGS));
 
+       g_object_class_install_property (
+               object_class,
+               PROP_SOFT_BUSY,
+               g_param_spec_boolean (
+                       "soft-busy",
+                       "Soft Busy",
+                       "Whether asynchronous actions are disabled",
+                       FALSE,
+                       G_PARAM_READABLE |
+                       G_PARAM_STATIC_STRINGS));
+
        g_object_class_install_property (
                object_class,
                PROP_EDITOR,
diff --git a/src/composer/e-msg-composer.h b/src/composer/e-msg-composer.h
index da82752fde..e62ca84297 100644
--- a/src/composer/e-msg-composer.h
+++ b/src/composer/e-msg-composer.h
@@ -207,6 +207,7 @@ void                e_msg_composer_save_focused_widget
 void           e_msg_composer_restore_focus_on_composer
                                                (EMsgComposer *composer);
 gboolean       e_msg_composer_is_busy          (EMsgComposer *composer);
+gboolean       e_msg_composer_is_soft_busy     (EMsgComposer *composer);
 gboolean       e_msg_composer_get_is_reply_or_forward
                                                (EMsgComposer *composer);
 void           e_msg_composer_set_is_reply_or_forward
diff --git a/src/modules/composer-autosave/e-composer-autosave.c 
b/src/modules/composer-autosave/e-composer-autosave.c
index 4797861881..6d151fd784 100644
--- a/src/modules/composer-autosave/e-composer-autosave.c
+++ b/src/modules/composer-autosave/e-composer-autosave.c
@@ -96,6 +96,7 @@ composer_autosave_timeout_cb (gpointer user_data)
 {
        EComposerAutosave *autosave;
        EExtensible *extensible;
+       EMsgComposer *composer;
 
        autosave = E_COMPOSER_AUTOSAVE (user_data);
 
@@ -103,6 +104,11 @@ composer_autosave_timeout_cb (gpointer user_data)
                return FALSE;
 
        extensible = e_extension_get_extensible (E_EXTENSION (autosave));
+       composer = E_MSG_COMPOSER (extensible);
+
+       /* Do not do anything when it's busy */
+       if (e_msg_composer_is_soft_busy (composer))
+               return FALSE;
 
        /* Cancel the previous snapshot if it's still in
         * progress and start a new snapshot operation. */
@@ -111,7 +117,7 @@ composer_autosave_timeout_cb (gpointer user_data)
        autosave->priv->cancellable = g_cancellable_new ();
 
        e_composer_save_snapshot (
-               E_MSG_COMPOSER (extensible),
+               composer,
                autosave->priv->cancellable,
                composer_autosave_finished_cb,
                g_object_ref (autosave));