[epiphany/mcatanzaro/CVE-2019-6251-Regressions] Do not trust URI during WEBKIT_LOAD_STARTED



commit 45118c69707fa6f993c065e6ebf5255e4c8f1515
Author: Michael Catanzaro <mcatanzaro gnome org>
Date:   Tue Aug 6 15:37:08 2019 -0500

    Do not trust URI during WEBKIT_LOAD_STARTED
    
    Since WebKit r243434 [1][2], the web view's URI property might not be
    updated during WEBKIT_LOAD_STARTED. For example, when on the
    about:overview page, if we click on any overview thumbnail, the URI is
    still ephy-about://overview at this point. WebKit internally knows the
    URI is different, but it is hiding the change from us until
    WEBKIT_LOAD_COMMITTED because it doesn't know if web content is
    maliciously attempting to spoof the URI. The URI is now only expected to
    be accurate if the load was initiated by API request, e.g.
    webkit_web_view_load_uri(), and our code here doesn't know anything
    about how the load was initiated, so we'd better not check the URI here
    at all.
    
    There were several regressions that I never noticed until today:
    
     (1) We freeze the history here improperly, since we incorrectly think
         that we are loading about:overview. Then the page we load doesn't
         make it into history.
    
     (2) For the same reason, we don't save a new snapshot of the page for
         the overview, resulting in stale snapshots persisting the next time
         the overview is opened.
    
     (3) We set the loading message in the floating statusbar to indicate
         that we are loading the currently-viewed page, rather than the page
         that is actually being loaded. To fix this, we can just set the
         label to "Loading...", without displaying any URL at all, until
         WEBKIT_LOAD_COMMITTED is reached.
    
    These bugs only occur when the load is initiated by web content, or by
    user interaction with web content. Loads triggered by API request should
    be fine.
    
    [1] https://trac.webkit.org/changeset/243434
    [2] https://bugs.webkit.org/show_bug.cgi?id=194208

 embed/ephy-web-view.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)
---
diff --git a/embed/ephy-web-view.c b/embed/ephy-web-view.c
index 39f2efdde..71285beee 100644
--- a/embed/ephy-web-view.c
+++ b/embed/ephy-web-view.c
@@ -1776,13 +1776,6 @@ restore_zoom_level (EphyWebView *view,
                                            (EphyHistoryJobCallback)get_host_for_url_cb, view);
 }
 
-/**
- * ephy_web_view_set_loading_message:
- * @view: an #EphyWebView
- * @address: the loading address
- *
- * Update @view's loading message
- **/
 static void
 ephy_web_view_set_loading_message (EphyWebView *view,
                                    const char  *address)
@@ -1804,11 +1797,20 @@ ephy_web_view_set_loading_message (EphyWebView *view,
 
     g_free (decoded_address);
     g_free (title);
+  } else {
+    view->loading_message = g_strdup (_("Loading…"));
   }
 
   g_object_notify_by_pspec (G_OBJECT (view), obj_properties[PROP_STATUS_MESSAGE]);
 }
 
+static void
+ephy_web_view_unset_loading_message (EphyWebView *view)
+{
+  g_clear_pointer (&view->loading_message, g_free);
+  g_object_notify_by_pspec (G_OBJECT (view), obj_properties[PROP_STATUS_MESSAGE]);
+}
+
 static void
 ephy_web_view_set_committed_location (EphyWebView *view,
                                       const char  *location)
@@ -1907,10 +1909,12 @@ load_changed_cb (WebKitWebView   *web_view,
 
   view->in_auth_dialog = 0;
 
+  /* Warning: the URI property may remain set to the URI of the
+   * previously-loaded page until WEBKIT_LOAD_COMMITTED! During
+   * WEBKIT_LOAD_STARTED, it may or may not match the URI being loaded.
+   */
   switch (load_event) {
     case WEBKIT_LOAD_STARTED: {
-      const char *loading_uri = NULL;
-
       view->load_failed = FALSE;
 
       if (view->snapshot_timeout_id) {
@@ -1918,16 +1922,11 @@ load_changed_cb (WebKitWebView   *web_view,
         view->snapshot_timeout_id = 0;
       }
 
-      loading_uri = webkit_web_view_get_uri (web_view);
-
-      if (ephy_embed_utils_is_no_show_address (loading_uri))
-        ephy_web_view_freeze_history (view);
-
+      /* XXX: what is this for? */
       if (view->address == NULL || view->address[0] == '\0')
-        ephy_web_view_set_address (view, loading_uri);
-
-      ephy_web_view_set_loading_message (view, loading_uri);
+        ephy_web_view_set_address (view, webkit_web_view_get_uri (web_view));
 
+      ephy_web_view_set_loading_message (view, NULL);
 
       if (!view->reader_loading) {
         g_clear_pointer (&view->reader_byline, g_free);
@@ -1951,6 +1950,9 @@ load_changed_cb (WebKitWebView   *web_view,
       update_security_status_for_committed_load (view, uri);
 
       /* History. */
+      if (ephy_embed_utils_is_no_show_address (uri))
+        ephy_web_view_freeze_history (view);
+
       if (!ephy_web_view_is_history_frozen (view)) {
         char *history_uri = NULL;
 
@@ -1985,7 +1987,7 @@ load_changed_cb (WebKitWebView   *web_view,
       break;
     }
     case WEBKIT_LOAD_FINISHED:
-      ephy_web_view_set_loading_message (view, NULL);
+      ephy_web_view_unset_loading_message (view);
 
       /* Ensure we load the icon for this web view, if available. */
       _ephy_web_view_update_icon (view);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]