- Revision
- 247025
- Author
- [email protected]
- Date
- 2019-07-01 14:55:03 -0700 (Mon, 01 Jul 2019)
Log Message
It should not be possible to trigger a load while in the middle of restoring a page in PageCache
https://bugs.webkit.org/show_bug.cgi?id=199190
<rdar://problem/52114552>
Reviewed by Brady Eidson.
Source/WebCore:
Test: http/tests/security/navigate-when-restoring-cached-page.html
* history/CachedFrame.cpp:
(WebCore::CachedFrame::open):
Stop attaching the cached document before calling FrameLoader::open() given that the previous document
is still attached to the frame at this point. This avoids having 2 documents attached to the same frame
during a short period of time.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::open):
We now attach the cached document to the frame *after* calling FrameLoader::clear(), which means that
the previous document now has been detached from this frame.
(WebCore::FrameLoader::detachChildren):
As per the HTML specification [1], an attempt to navigate should fail if the prompt to unload algorithm
is being run for the active document of browsingContext. Note that the "prompt to unload" algorithm [2]
includes firing the 'unload' event in the current document and in all the documents in the subframes.
As a result, FrameLoader::detachChildren() is the right prevent such navigations. We were actually trying
to do this via the SubframeLoadingDisabler stack variable inside detachChildren(). The issue is that this
only prevents navigation in the subframes (i.e. <iframe> elements), not the main frame. As a result,
script would be able to navigate the top-frame even though detachChildren() is being called on the top
frame. To address the issue, I now create a NavigationDisabler variable in the scope of detachChildren()
when detachChildren() is called on the top frame. NavigationDisabler prevents all navigations within the
page, including navigations on the main/top frame.
[1] https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate
[2] https://html.spec.whatwg.org/multipage/browsing-the-web.html#prompt-to-unload-a-document
LayoutTests:
Add layout test coverage.
* http/tests/security/navigate-when-restoring-cached-page-expected.txt: Added.
* http/tests/security/navigate-when-restoring-cached-page.html: Added.
* http/tests/security/resources/navigate-when-restoring-cached-page-frame.html: Added.
* http/tests/security/resources/navigate-when-restoring-cached-page-victim.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (247024 => 247025)
--- trunk/LayoutTests/ChangeLog 2019-07-01 21:25:57 UTC (rev 247024)
+++ trunk/LayoutTests/ChangeLog 2019-07-01 21:55:03 UTC (rev 247025)
@@ -1,3 +1,18 @@
+2019-07-01 Chris Dumez <[email protected]>
+
+ It should not be possible to trigger a load while in the middle of restoring a page in PageCache
+ https://bugs.webkit.org/show_bug.cgi?id=199190
+ <rdar://problem/52114552>
+
+ Reviewed by Brady Eidson.
+
+ Add layout test coverage.
+
+ * http/tests/security/navigate-when-restoring-cached-page-expected.txt: Added.
+ * http/tests/security/navigate-when-restoring-cached-page.html: Added.
+ * http/tests/security/resources/navigate-when-restoring-cached-page-frame.html: Added.
+ * http/tests/security/resources/navigate-when-restoring-cached-page-victim.html: Added.
+
2019-07-01 Truitt Savell <[email protected]>
Unreviewed, rolling out r246844.
Added: trunk/LayoutTests/http/tests/security/navigate-when-restoring-cached-page-expected.txt (0 => 247025)
--- trunk/LayoutTests/http/tests/security/navigate-when-restoring-cached-page-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/navigate-when-restoring-cached-page-expected.txt 2019-07-01 21:55:03 UTC (rev 247025)
@@ -0,0 +1 @@
+This test passes if it does not print an ALERT with "secret data".
Added: trunk/LayoutTests/http/tests/security/navigate-when-restoring-cached-page.html (0 => 247025)
--- trunk/LayoutTests/http/tests/security/navigate-when-restoring-cached-page.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/navigate-when-restoring-cached-page.html 2019-07-01 21:55:03 UTC (rev 247025)
@@ -0,0 +1,99 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This test passes if it does not print an ALERT with "secret data".</p>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+ testRunner.setCanOpenWindows();
+}
+
+function createURL(data, type = 'text/html') {
+ return URL.createObjectURL(new Blob([data], {type: type}));
+}
+
+victim_url = 'http://localhost:8000/security/resources/navigate-when-restoring-cached-page-victim.html?' + Math.random();
+
+cache_frame = document.body.appendChild(document.createElement('iframe'));
+cache_frame.style.display = 'none';
+cache_frame.src = ""
+
+function runTest()
+{
+ if (window.testRunner && !testRunner.isWebKit2) {
+ // This test does not run (times out) on WebKit1.
+ testRunner.notifyDone();
+ return;
+ }
+
+ showModalDialog(createURL(`
+ <script>
+ _onmessage_ = e => {
+ w.close();
+ if (window.testRunner)
+ testRunner.abortModal();
+ alert(e.data);
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+
+ ${createURL}
+
+ w = open('about:blank');
+ if (w == null) {
+ alert('coudn\\'t open enough windows, try again');
+ close();
+ if (window.testRunner) {
+ testRunner.abortModal();
+ testRunner.notifyDone();
+ }
+ }
+ w.history.pushState('', '');
+
+ cached_doc = w.document;
+ setTimeout(() => {
+ w.location = 'about:blank';
+ interval_id = setInterval(() => {
+ if (cached_doc == w.document)
+ return;
+
+ clearInterval(interval_id);
+
+ canvas = w.document.body.appendChild(document.createElement('canvas'));
+ context = canvas.getContext('webgl');
+
+ child_frame = w.document.body.appendChild(document.createElement('iframe'));
+ child_frame.contentWindow._onunload_ = () => {
+ child_frame.contentWindow._onunload_ = null;
+
+ a = w.document.createElement('a');
+ a.href = '';
+ a.click();
+
+ showModalDialog(createURL('<script>timeout = 100; setInterval(() => { if (!--timeout || opener.child_loaded) { close(); if (window.testRunner) testRunner.abortModal(); } }, 1)</sc' + 'ript>'));
+
+ if (!window.child_loaded && window.testRunner) {
+ testRunner.abortModal();
+ testRunner.notifyDone();
+ }
+ }
+
+ w.history.back();
+ }, 0);
+ }, 0);
+ </scr` + `ipt>`));
+}
+
+_onload_ = function() {
+ setTimeout(() => {
+ if (window.internals)
+ internals.withUserGesture(runTest);
+ else
+ _onclick_ = runTest;
+ }, 0);
+}
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/security/resources/navigate-when-restoring-cached-page-frame.html (0 => 247025)
--- trunk/LayoutTests/http/tests/security/resources/navigate-when-restoring-cached-page-frame.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/resources/navigate-when-restoring-cached-page-frame.html 2019-07-01 21:55:03 UTC (rev 247025)
@@ -0,0 +1,12 @@
+<script>
+
+if (parent.opener) {
+ _onunload_ = () => {
+ a = parent.opener.cached_doc.body.appendChild(document.createElement('form'));
+ a.action = '';
+ a.submit();
+ }
+
+ parent.opener.child_loaded = true;
+}
+</script>
Added: trunk/LayoutTests/http/tests/security/resources/navigate-when-restoring-cached-page-victim.html (0 => 247025)
--- trunk/LayoutTests/http/tests/security/resources/navigate-when-restoring-cached-page-victim.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/resources/navigate-when-restoring-cached-page-victim.html 2019-07-01 21:55:03 UTC (rev 247025)
@@ -0,0 +1,2 @@
+<h1>secret data</h1>
+<iframe src=""
Modified: trunk/Source/WebCore/ChangeLog (247024 => 247025)
--- trunk/Source/WebCore/ChangeLog 2019-07-01 21:25:57 UTC (rev 247024)
+++ trunk/Source/WebCore/ChangeLog 2019-07-01 21:55:03 UTC (rev 247025)
@@ -1,3 +1,39 @@
+2019-07-01 Chris Dumez <[email protected]>
+
+ It should not be possible to trigger a load while in the middle of restoring a page in PageCache
+ https://bugs.webkit.org/show_bug.cgi?id=199190
+ <rdar://problem/52114552>
+
+ Reviewed by Brady Eidson.
+
+ Test: http/tests/security/navigate-when-restoring-cached-page.html
+
+ * history/CachedFrame.cpp:
+ (WebCore::CachedFrame::open):
+ Stop attaching the cached document before calling FrameLoader::open() given that the previous document
+ is still attached to the frame at this point. This avoids having 2 documents attached to the same frame
+ during a short period of time.
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::open):
+ We now attach the cached document to the frame *after* calling FrameLoader::clear(), which means that
+ the previous document now has been detached from this frame.
+
+ (WebCore::FrameLoader::detachChildren):
+ As per the HTML specification [1], an attempt to navigate should fail if the prompt to unload algorithm
+ is being run for the active document of browsingContext. Note that the "prompt to unload" algorithm [2]
+ includes firing the 'unload' event in the current document and in all the documents in the subframes.
+ As a result, FrameLoader::detachChildren() is the right prevent such navigations. We were actually trying
+ to do this via the SubframeLoadingDisabler stack variable inside detachChildren(). The issue is that this
+ only prevents navigation in the subframes (i.e. <iframe> elements), not the main frame. As a result,
+ script would be able to navigate the top-frame even though detachChildren() is being called on the top
+ frame. To address the issue, I now create a NavigationDisabler variable in the scope of detachChildren()
+ when detachChildren() is called on the top frame. NavigationDisabler prevents all navigations within the
+ page, including navigations on the main/top frame.
+
+ [1] https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate
+ [2] https://html.spec.whatwg.org/multipage/browsing-the-web.html#prompt-to-unload-a-document
+
2019-07-01 Truitt Savell <[email protected]>
Unreviewed, rolling out r246844.
Modified: trunk/Source/WebCore/history/CachedFrame.cpp (247024 => 247025)
--- trunk/Source/WebCore/history/CachedFrame.cpp 2019-07-01 21:25:57 UTC (rev 247024)
+++ trunk/Source/WebCore/history/CachedFrame.cpp 2019-07-01 21:55:03 UTC (rev 247025)
@@ -217,8 +217,6 @@
if (!m_isMainFrame)
m_view->frame().page()->incrementSubframeCount();
- m_document->attachToCachedFrame(*this);
-
m_view->frame().loader().open(*this);
}
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (247024 => 247025)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2019-07-01 21:25:57 UTC (rev 247024)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2019-07-01 21:55:03 UTC (rev 247025)
@@ -2308,6 +2308,7 @@
clear(document, true, true, cachedFrame.isMainFrame());
+ document->attachToCachedFrame(cachedFrame);
document->setPageCacheState(Document::NotInPageCache);
m_needsClear = true;
@@ -2675,6 +2676,13 @@
// https://html.spec.whatwg.org/multipage/browsers.html#unload-a-document
IgnoreOpensDuringUnloadCountIncrementer ignoreOpensDuringUnloadCountIncrementer(m_frame.document());
+ // detachChildren() will fire the unload event in each subframe and the
+ // HTML specification states that navigations should be prevented during the prompt to unload algorithm:
+ // https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate
+ std::unique_ptr<NavigationDisabler> navigationDisabler;
+ if (m_frame.isMainFrame())
+ navigationDisabler = std::make_unique<NavigationDisabler>(&m_frame);
+
// Any subframe inserted by unload event handlers executed in the loop below will not get unloaded
// because we create a copy of the subframes list before looping. Therefore, it would be unsafe to
// allow loading of subframes at this point.
