Hi all,

I noticed that if a HTML message contains both attached (“cid:…”) and external 
(“http://…”, “https://…”, …) image links, Balsa loads /all/ images, both the 
attached *and* the external ones.  The reason: whenever an attached image is 
referenced, Balsa enables Webkit's auto-load feature.  The info bar for 
selecting whether the external images shall be loaded appears briefly and is 
then removed automatically.

This behaviour should be considered as security bug, as it allows an attacker 
to enable on not only trivial attacks against the user's privacy like web bugs, 
but also work around the EFail protection.

Unfortunately, I didn't find a way to selectively load images (anyone knows a trick to 
achieve it?); whenever webkit_settings_set_auto_load_images() is set to TRUE, Webkit will 
load /all/ of them, not calling the "decide-policy" callback.  Thus, the only 
fix I found (attached) is to completely disable loading images in this case, i.e. the 
auto-load is enabled iff attached, but no external ones have been detected.  The drawback 
is that the attached images are also shown only if the user activates images.

Additionally, since version 2.20, the 'web-process-crashed' signal is 
deprecated and has been replaced by 'web-process-terminated', with a slightly 
different callback signature, which is also addressed in the patch.

Opinions?

Cheers,
Albrecht.
diff --git a/libbalsa/html.c b/libbalsa/html.c
index b44472ae9..f4a2f2ef6 100644
--- a/libbalsa/html.c
+++ b/libbalsa/html.c
@@ -471,6 +471,31 @@ lbh_resource_load_started_cb(WebKitWebView     * web_view,
                      G_CALLBACK(lbh_resource_notify_response_cb), data);
 }
 
+#if WEBKIT_CHECK_VERSION(2,20,0)
+/*
+ * Callback for the "web-process-terminated" signal
+ */
+static void
+lbh_web_process_terminated_cb(WebKitWebView                     *web_view,
+               	   	   	   	  WebKitWebProcessTerminationReason  reason,
+							  gpointer                           user_data)
+{
+	const gchar *reason_str;
+
+	switch (reason) {
+	case WEBKIT_WEB_PROCESS_CRASHED:
+		reason_str = "crashed";
+		break;
+	case WEBKIT_WEB_PROCESS_EXCEEDED_MEMORY_LIMIT:
+		reason_str = "exceeded memory limit";
+		break;
+	default:
+		reason_str = "unknown";
+		break;
+	}
+	g_warning("webkit process terminated abnormally: %s", reason_str);
+}
+#else
 /*
  * Callback for the "web-process-crashed" signal
  */
@@ -481,6 +506,7 @@ lbh_web_process_crashed_cb(WebKitWebView * web_view,
     g_debug("%s", __func__);
     return FALSE;
 }
+#endif
 
 /*
  * WebKitURISchemeRequestCallback for "cid:" URIs
@@ -567,6 +593,8 @@ libbalsa_html_new(LibBalsaMessageBody * body,
         "<[^>]*src\\s*=\\s*['\"]?\\s*cid:";
     static const gchar src_regex[] =
         "<[^>]*src\\s*=\\s*['\"]?\\s*[^c][^i][^d][^:]";
+    gboolean have_src_cid;
+    gboolean have_src_oth;
 
     len = lbh_get_body_content_utf8(body, &text);
     if (len < 0)
@@ -604,14 +632,15 @@ libbalsa_html_new(LibBalsaMessageBody * body,
         g_debug("%s registered cid: scheme", __func__);
     }
 
+    have_src_cid = g_regex_match_simple(cid_regex, text, G_REGEX_CASELESS, 0);
+    have_src_oth = g_regex_match_simple(src_regex, text, G_REGEX_CASELESS, 0);
+
     settings = webkit_web_view_get_settings(web_view);
     webkit_settings_set_enable_plugins(settings, FALSE);
     webkit_settings_set_enable_javascript(settings, FALSE);
 	webkit_settings_set_enable_java(settings, FALSE);
 	webkit_settings_set_enable_hyperlink_auditing(settings, TRUE);
-    webkit_settings_set_auto_load_images
-        (settings,
-         g_regex_match_simple(cid_regex, text, G_REGEX_CASELESS, 0));
+    webkit_settings_set_auto_load_images(settings, have_src_cid && !have_src_oth);
 
     g_signal_connect(web_view, "mouse-target-changed",
                      G_CALLBACK(lbh_mouse_target_changed_cb), info);
@@ -619,8 +648,13 @@ libbalsa_html_new(LibBalsaMessageBody * body,
                      G_CALLBACK(lbh_decide_policy_cb), info);
     g_signal_connect(web_view, "resource-load-started",
                      G_CALLBACK(lbh_resource_load_started_cb), info);
-    g_signal_connect(web_view, "web-process-crashed",
+#if WEBKIT_CHECK_VERSION(2,20,0)
+	g_signal_connect(web_view, "web-process-terminated",
+                     G_CALLBACK(lbh_web_process_terminated_cb), info);
+#else
+	g_signal_connect(web_view, "web-process-crashed",
                      G_CALLBACK(lbh_web_process_crashed_cb), info);
+#endif
     g_signal_connect(web_view, "context-menu",
                      G_CALLBACK(lbh_context_menu_cb), info);
 
@@ -629,7 +663,7 @@ libbalsa_html_new(LibBalsaMessageBody * body,
     gtk_box_pack_end(GTK_BOX(vbox), widget, TRUE, TRUE, 0);
 
     /* Simple check for possible resource requests: */
-    if (g_regex_match_simple(src_regex, text, G_REGEX_CASELESS, 0)) {
+    if (have_src_oth) {
         info->info_bar = lbh_info_bar(info);
         gtk_box_pack_start(GTK_BOX(vbox), info->info_bar, FALSE, FALSE, 0);
         g_debug("%s shows info_bar", __func__);

Attachment: pgp7sZ_ojfVke.pgp
Description: PGP signature

_______________________________________________
balsa-list mailing list
[email protected]
https://mail.gnome.org/mailman/listinfo/balsa-list

Reply via email to