WebKit Bugzilla
Attachment 361034 Details for
Bug 194208
: [WPE][GTK] URI spoofing when JS redirects page to something that takes a long time to load
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-194208-20190203211608.patch (text/plain), 6.53 KB, created by
Michael Catanzaro
on 2019-02-03 19:16:09 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Michael Catanzaro
Created:
2019-02-03 19:16:09 PST
Size:
6.53 KB
patch
obsolete
>Subversion Revision: 240850 >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index f789ec2ed670cea373589ba818df877c7a95944f..00d68c777511ebe70240af6b95d99a98db9e6819 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,33 @@ >+2019-02-03 Michael Catanzaro <mcatanzaro@igalia.com> >+ >+ (CVE-2019-6251) [WPE][GTK] URI issue when JS redirects page to something that takes a long time to load >+ https://bugs.webkit.org/show_bug.cgi?id=194208 >+ <rdar://problem/47776021> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ We must not allow URI updates between committed and finished, because this is when JS could >+ trigger redirects that will not ever complete, for the purpose of displaying problematic >+ content with a trusted URI in the address bar. The user's only indication that something >+ might be wrong would be a spinner. >+ >+ I tried writing an API test for this, but it's not possible because we want to unfreeze the >+ URI immediately before emitting finished, of course, but then a test has no way to know if >+ it's happening too soon or not, because the test just knows the load hasn't finished yet. >+ >+ My solution relies on the fact that finished is now always emitted even if the load never >+ finishes, but this is also a hindrance, because the problematic page could circumvent this >+ new protection by just starting a new load (causing the previous one to "finish"), so we >+ also need to notify the web view when a new load occurs before the previous one finished, >+ and reset the URI in this case. (This part might be testable, but I haven't tried.) >+ >+ * UIProcess/API/glib/WebKitNavigationClient.cpp: >+ (NavigationClient::NavigationClient): >+ * UIProcess/API/glib/WebKitWebView.cpp: >+ (webkitWebViewLoadChanged): >+ (webkitWebViewResetToLastCommittedURI): >+ * UIProcess/API/glib/WebKitWebViewPrivate.h: >+ > 2019-02-03 Michael Catanzaro <mcatanzaro@igalia.com> > > [WPE][GTK] Load events may occur in unexpected order when JS redirects page before subresource load finishes >diff --git a/Source/WebKit/UIProcess/API/glib/WebKitNavigationClient.cpp b/Source/WebKit/UIProcess/API/glib/WebKitNavigationClient.cpp >index fc1c943123c5d2e48d7fd7cbdac8749f7e1b478a..a5d0de83b44e5eb9c89396eec807276b711a3aef 100644 >--- a/Source/WebKit/UIProcess/API/glib/WebKitNavigationClient.cpp >+++ b/Source/WebKit/UIProcess/API/glib/WebKitNavigationClient.cpp >@@ -101,7 +101,13 @@ class NavigationClient : public API::NavigationClient { > public: > explicit NavigationClient(WebKitWebView* webView) > : m_webView(webView) >- , m_navigationStateTracker([this] { webkitWebViewLoadChanged(m_webView, WEBKIT_LOAD_FINISHED); }) >+ , m_navigationStateTracker([this] { >+ // We have to reset the URI here because a malicious page could >+ // attempt to spoof a URI on a legitimate host if it knows that >+ // the load won't complete. >+ webkitWebViewResetToLastCommittedURI(m_webView); >+ webkitWebViewLoadChanged(m_webView, WEBKIT_LOAD_FINISHED); >+ }) > { > } > >diff --git a/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp b/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp >index 2979b8a5238c7e8d635bdb440f52948c5fc8534f..775db5594c466e2613084fcc7be8a45373711ec7 100644 >--- a/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp >+++ b/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp >@@ -246,9 +246,11 @@ struct _WebKitWebViewPrivate { > CString title; > CString customTextEncoding; > CString activeURI; >- bool isLoading; >- bool isEphemeral; >- bool isControlledByAutomation; >+ CString lastCommittedURI; >+ bool uriUpdatesAreBlocked : 1; >+ bool isLoading : 1; >+ bool isEphemeral : 1; >+ bool isControlledByAutomation : 1; > > std::unique_ptr<PageLoadStateObserver> loadObserver; > >@@ -360,7 +362,8 @@ private: > void didChangeActiveURL() override > { > m_webView->priv->activeURI = getPage(m_webView).pageLoadState().activeURL().utf8(); >- g_object_notify(G_OBJECT(m_webView), "uri"); >+ if (!m_webView->priv->uriUpdatesAreBlocked) >+ g_object_notify(G_OBJECT(m_webView), "uri"); > g_object_thaw_notify(G_OBJECT(m_webView)); > } > >@@ -2070,16 +2073,20 @@ void webkitWebViewLoadChanged(WebKitWebView* webView, WebKitLoadEvent loadEvent) > priv->loadingResourcesMap.clear(); > priv->mainResource = nullptr; > break; >-#if PLATFORM(GTK) > case WEBKIT_LOAD_COMMITTED: { >+#if PLATFORM(GTK) > WebKitFaviconDatabase* database = webkit_web_context_get_favicon_database(priv->context.get()); > GUniquePtr<char> faviconURI(webkit_favicon_database_get_favicon_uri(database, priv->activeURI.data())); > webkitWebViewUpdateFaviconURI(webView, faviconURI.get()); >+#endif >+ priv->uriUpdatesAreBlocked = true; > break; > } >-#endif > case WEBKIT_LOAD_FINISHED: > webkitWebViewCancelAuthenticationRequest(webView); >+ priv->uriUpdatesAreBlocked = false; >+ if (priv->activeURI != priv->lastCommittedURI) >+ g_object_notify(G_OBJECT(webView), "uri"); > break; > default: > break; >@@ -2088,6 +2095,11 @@ void webkitWebViewLoadChanged(WebKitWebView* webView, WebKitLoadEvent loadEvent) > g_signal_emit(webView, signals[LOAD_CHANGED], 0, loadEvent); > } > >+void webkitWebViewResetToLastCommittedURI(WebKitWebView* webView) >+{ >+ webView->priv->activeURI = webView->priv->lastCommittedURI; >+} >+ > void webkitWebViewLoadFailed(WebKitWebView* webView, WebKitLoadEvent loadEvent, const char* failingURI, GError *error) > { > webkitWebViewCancelAuthenticationRequest(webView); >diff --git a/Source/WebKit/UIProcess/API/glib/WebKitWebViewPrivate.h b/Source/WebKit/UIProcess/API/glib/WebKitWebViewPrivate.h >index e9ef60c577c4541b872092d732444a7eabad9a4d..fb032439e1a99312c0d6fee1e0cce1818b77f178 100644 >--- a/Source/WebKit/UIProcess/API/glib/WebKitWebViewPrivate.h >+++ b/Source/WebKit/UIProcess/API/glib/WebKitWebViewPrivate.h >@@ -42,6 +42,7 @@ > void webkitWebViewCreatePage(WebKitWebView*, Ref<API::PageConfiguration>&&); > WebKit::WebPageProxy& webkitWebViewGetPage(WebKitWebView*); > void webkitWebViewLoadChanged(WebKitWebView*, WebKitLoadEvent); >+void webkitWebViewResetToLastCommittedURI(WebKitWebView*); > void webkitWebViewLoadFailed(WebKitWebView*, WebKitLoadEvent, const char* failingURI, GError*); > void webkitWebViewLoadFailedWithTLSErrors(WebKitWebView*, const char* failingURI, GError*, GTlsCertificateFlags, GTlsCertificate*); > #if PLATFORM(GTK)
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194208
:
361034
|
365721
|
365808