Diff
Modified: trunk/LayoutTests/ChangeLog (270372 => 270373)
--- trunk/LayoutTests/ChangeLog 2020-12-02 23:00:55 UTC (rev 270372)
+++ trunk/LayoutTests/ChangeLog 2020-12-02 23:01:29 UTC (rev 270373)
@@ -1,3 +1,17 @@
+2020-12-02 Chris Dumez <[email protected]>
+
+ iframe with `sandbox=allow-top-navigation-by-user-activation` can navigate top frame when the user interacts with an iframe from another origin
+ https://bugs.webkit.org/show_bug.cgi?id=219413
+ <rdar://problem/64887657>
+
+ Reviewed by Geoffrey Garen.
+
+ Add layout test coverage.
+
+ * http/tests/security/block-top-level-navigations-by-sandboxed-iframe-with-propagated-user-gesture-expected.txt: Added.
+ * http/tests/security/block-top-level-navigations-by-sandboxed-iframe-with-propagated-user-gesture.html: Added.
+ * http/tests/security/resources/navigate-top-level-frame-to-failure-page-via-message-handler.html: Added.
+
2020-12-02 Ryan Haddad <[email protected]>
[macOS] imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiobuffer-interface/ctor-audiobuffer.html is a flaky failure
Added: trunk/LayoutTests/http/tests/security/block-top-level-navigations-by-sandboxed-iframe-with-propagated-user-gesture-expected.txt (0 => 270373)
--- trunk/LayoutTests/http/tests/security/block-top-level-navigations-by-sandboxed-iframe-with-propagated-user-gesture-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/block-top-level-navigations-by-sandboxed-iframe-with-propagated-user-gesture-expected.txt 2020-12-02 23:01:29 UTC (rev 270373)
@@ -0,0 +1,13 @@
+CONSOLE MESSAGE: Unsafe _javascript_ attempt to initiate navigation for frame with URL 'http://127.0.0.1:8000/security/block-top-level-navigations-by-sandboxed-iframe-with-propagated-user-gesture.html' from frame with URL 'http://localhost:8000/security/resources/navigate-top-level-frame-to-failure-page-via-message-handler.html'. The frame attempting navigation of the top-level window is sandboxed, but the 'allow-top-navigation' flag is not set.
+
+CONSOLE MESSAGE: SecurityError: The operation is insecure.
+Test blocking of top-level navigations by an iframe with `sandbox=allow-top-navigation-by-user-activation` when the user gesture is propagated from another context.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS All navigations by subframes have been blocked
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/security/block-top-level-navigations-by-sandboxed-iframe-with-propagated-user-gesture.html (0 => 270373)
--- trunk/LayoutTests/http/tests/security/block-top-level-navigations-by-sandboxed-iframe-with-propagated-user-gesture.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/block-top-level-navigations-by-sandboxed-iframe-with-propagated-user-gesture.html 2020-12-02 23:01:29 UTC (rev 270373)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Test blocking of top-level navigations by an iframe with `sandbox=allow-top-navigation-by-user-activation` when the user gesture is propagated from another context.");
+jsTestIsAsync = true;
+_onload_ = () => {
+ setTimeout(() => {
+ internals.withUserGesture(() => {
+ document.getElementsByTagName('iframe')[0].contentWindow.postMessage("foo", "*");
+ });
+ setTimeout(() => {
+ testPassed("All navigations by subframes have been blocked");
+ finishJSTest();
+ }, 100);
+ }, 10);
+}
+</script>
+<iframe id="testFrame" src="" sandbox="allow-forms allow-pointer-lock allow-popups-to-escape-sandbox allow-popups allow-same-origin allow-scripts allow-top-navigation-by-user-activation"></iframe>
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-failure-page-via-message-handler.html (0 => 270373)
--- trunk/LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-failure-page-via-message-handler.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/resources/navigate-top-level-frame-to-failure-page-via-message-handler.html 2020-12-02 23:01:29 UTC (rev 270373)
@@ -0,0 +1,9 @@
+<html>
+<body>
+<script>
+_onmessage_ = (e) => {
+ top.location = "http://127.0.0.1:8000/security/resources/should-not-have-loaded.html";
+}
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (270372 => 270373)
--- trunk/Source/WebCore/ChangeLog 2020-12-02 23:00:55 UTC (rev 270372)
+++ trunk/Source/WebCore/ChangeLog 2020-12-02 23:01:29 UTC (rev 270373)
@@ -1,5 +1,35 @@
2020-12-02 Chris Dumez <[email protected]>
+ iframe with `sandbox=allow-top-navigation-by-user-activation` can navigate top frame when the user interacts with an iframe from another origin
+ https://bugs.webkit.org/show_bug.cgi?id=219413
+ <rdar://problem/64887657>
+
+ Reviewed by Geoffrey Garen.
+
+ An iframe with `sandbox=allow-top-navigation-by-user-activation` can navigate the top frame when the user
+ interacts with an frame from another origin. This is not strict enough and does not match the behavior of
+ Chrome.
+
+ In Chrome, the user activation is only valid for the purpose of navigation if the user interacted with either:
+ - The iframe triggering the navigation
+ - A descendant iframe of the iframe triggering the navigation
+ - A frame from the same origin as the iframe triggering the navigation
+
+ This patch aligns our behavior with Chrome's.
+
+ Test: http/tests/security/block-top-level-navigations-by-sandboxed-iframe-with-propagated-user-gesture.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::canNavigateInternal):
+ * dom/UserGestureIndicator.cpp:
+ (WebCore::UserGestureToken::UserGestureToken):
+ (WebCore::UserGestureToken::isValidForDocument const):
+ (WebCore::UserGestureIndicator::processingUserGesture):
+ * dom/UserGestureIndicator.h:
+ (WebCore::UserGestureToken::create):
+
+2020-12-02 Chris Dumez <[email protected]>
+
Block suspicious top level navigations by iframes even if sandbox=allow-top-navigation is specified
https://bugs.webkit.org/show_bug.cgi?id=219408
<rdar://problem/71049726>
Modified: trunk/Source/WebCore/dom/Document.cpp (270372 => 270373)
--- trunk/Source/WebCore/dom/Document.cpp 2020-12-02 23:00:55 UTC (rev 270372)
+++ trunk/Source/WebCore/dom/Document.cpp 2020-12-02 23:01:29 UTC (rev 270373)
@@ -3477,8 +3477,11 @@
if (!isSandboxed(SandboxTopNavigation) && &targetFrame == &m_frame->tree().top())
return true;
+ // The user gesture only relaxes permissions for the purpose of navigating if its impacts the current document.
+ bool isProcessingUserGestureForDocument = UserGestureIndicator::processingUserGesture(m_frame->document());
+
// ii. A frame can navigate its top ancestor when its 'allow-top-navigation-by-user-activation' flag is set and navigation is triggered by user activation.
- if (!isSandboxed(SandboxTopNavigationByUserActivation) && UserGestureIndicator::processingUserGesture() && &targetFrame == &m_frame->tree().top())
+ if (!isSandboxed(SandboxTopNavigationByUserActivation) && isProcessingUserGestureForDocument && &targetFrame == &m_frame->tree().top())
return true;
// iii. A sandboxed frame can always navigate its descendants.
@@ -3495,14 +3498,13 @@
// 2. Otherwise, if B is a top-level browsing context, and is one of the ancestor browsing contexts of A, then:
if (m_frame != &targetFrame && &targetFrame == &m_frame->tree().top()) {
- bool triggeredByUserActivation = UserGestureIndicator::processingUserGesture();
// 1. If this algorithm is triggered by user activation and A's active document's active sandboxing flag set has its sandboxed top-level navigation with user activation browsing context flag set, then abort these steps negatively.
- if (triggeredByUserActivation && isSandboxed(SandboxTopNavigationByUserActivation)) {
+ if (isProcessingUserGestureForDocument && isSandboxed(SandboxTopNavigationByUserActivation)) {
printNavigationErrorMessage(targetFrame, url(), "The frame attempting navigation of the top-level window is sandboxed, but the 'allow-top-navigation-by-user-activation' flag is not set and navigation is not triggered by user activation."_s);
return false;
}
// 2. Otherwise, If this algorithm is not triggered by user activation and A's active document's active sandboxing flag set has its sandboxed top-level navigation without user activation browsing context flag set, then abort these steps negatively.
- if (!triggeredByUserActivation && isSandboxed(SandboxTopNavigation)) {
+ if (!isProcessingUserGestureForDocument && isSandboxed(SandboxTopNavigation)) {
printNavigationErrorMessage(targetFrame, url(), "The frame attempting navigation of the top-level window is sandboxed, but the 'allow-top-navigation' flag is not set."_s);
return false;
}
Modified: trunk/Source/WebCore/dom/UserGestureIndicator.cpp (270372 => 270373)
--- trunk/Source/WebCore/dom/UserGestureIndicator.cpp 2020-12-02 23:00:55 UTC (rev 270372)
+++ trunk/Source/WebCore/dom/UserGestureIndicator.cpp 2020-12-02 23:01:29 UTC (rev 270373)
@@ -30,6 +30,7 @@
#include "Document.h"
#include "Frame.h"
#include "ResourceLoadObserver.h"
+#include "SecurityOrigin.h"
#include <wtf/MainThread.h>
#include <wtf/NeverDestroyed.h>
#include <wtf/Optional.h>
@@ -43,6 +44,34 @@
return token;
}
+UserGestureToken::UserGestureToken(ProcessingUserGestureState state, UserGestureType gestureType, Document* document)
+ : m_state(state)
+ , m_gestureType(gestureType)
+{
+ if (!document || !processingUserGesture())
+ return;
+
+ // User gesture is valid for the document that received the user gesture, all of its ancestors
+ // as well as all same-origin documents on the page.
+ m_documentsImpactedByUserGesture.add(*document);
+
+ auto* documentFrame = document->frame();
+ if (!documentFrame)
+ return;
+
+ for (auto* ancestorFrame = documentFrame->tree().parent(); ancestorFrame; ancestorFrame = ancestorFrame->tree().parent()) {
+ if (auto* ancestorDocument = ancestorFrame->document())
+ m_documentsImpactedByUserGesture.add(ancestorDocument);
+ }
+
+ auto& documentOrigin = document->securityOrigin();
+ for (auto* frame = &documentFrame->tree().top(); frame; frame = frame->tree().traverseNext()) {
+ auto* frameDocument = frame->document();
+ if (frameDocument && documentOrigin.canAccess(frameDocument->securityOrigin()))
+ m_documentsImpactedByUserGesture.add(*frameDocument);
+ }
+}
+
UserGestureToken::~UserGestureToken()
{
for (auto& observer : m_destructionObservers)
@@ -60,6 +89,11 @@
maxIntervalForUserGestureForwardingForFetch = WTFMove(value);
}
+bool UserGestureToken::isValidForDocument(Document& document) const
+{
+ return m_documentsImpactedByUserGesture.contains(document);
+}
+
UserGestureIndicator::UserGestureIndicator(Optional<ProcessingUserGestureState> state, Document* document, UserGestureType gestureType, ProcessInteractionStyle processInteractionStyle)
: m_previousToken { currentToken() }
{
@@ -66,7 +100,7 @@
ASSERT(isMainThread());
if (state)
- currentToken() = UserGestureToken::create(state.value(), gestureType);
+ currentToken() = UserGestureToken::create(state.value(), gestureType, document);
if (document && currentToken()->processingUserGesture() && state) {
document->updateLastHandledUserGestureTimestamp(currentToken()->startTime());
@@ -123,12 +157,15 @@
return currentToken();
}
-bool UserGestureIndicator::processingUserGesture()
+bool UserGestureIndicator::processingUserGesture(Document* document)
{
if (!isMainThread())
return false;
- return currentToken() ? currentToken()->processingUserGesture() : false;
+ if (!currentToken() || !currentToken()->processingUserGesture())
+ return false;
+
+ return !document || currentToken()->isValidForDocument(*document);
}
bool UserGestureIndicator::processingUserGestureForMedia()
Modified: trunk/Source/WebCore/dom/UserGestureIndicator.h (270372 => 270373)
--- trunk/Source/WebCore/dom/UserGestureIndicator.h 2020-12-02 23:00:55 UTC (rev 270372)
+++ trunk/Source/WebCore/dom/UserGestureIndicator.h 2020-12-02 23:01:29 UTC (rev 270373)
@@ -31,6 +31,7 @@
#include <wtf/Noncopyable.h>
#include <wtf/RefCounted.h>
#include <wtf/Vector.h>
+#include <wtf/WeakHashSet.h>
#include <wtf/WeakPtr.h>
namespace WebCore {
@@ -51,9 +52,9 @@
static const Seconds& maximumIntervalForUserGestureForwardingForFetch();
WEBCORE_EXPORT static void setMaximumIntervalForUserGestureForwardingForFetchForTesting(Seconds);
- static Ref<UserGestureToken> create(ProcessingUserGestureState state, UserGestureType gestureType)
+ static Ref<UserGestureToken> create(ProcessingUserGestureState state, UserGestureType gestureType, Document* document = nullptr)
{
- return adoptRef(*new UserGestureToken(state, gestureType));
+ return adoptRef(*new UserGestureToken(state, gestureType, document));
}
WEBCORE_EXPORT ~UserGestureToken();
@@ -101,16 +102,15 @@
MonotonicTime startTime() const { return m_startTime; }
+ bool isValidForDocument(Document&) const;
+
private:
- UserGestureToken(ProcessingUserGestureState state, UserGestureType gestureType)
- : m_state(state)
- , m_gestureType(gestureType)
- {
- }
+ UserGestureToken(ProcessingUserGestureState, UserGestureType, Document*);
ProcessingUserGestureState m_state = NotProcessingUserGesture;
Vector<WTF::Function<void (UserGestureToken&)>> m_destructionObservers;
UserGestureType m_gestureType;
+ WeakHashSet<Document> m_documentsImpactedByUserGesture;
DOMPasteAccessPolicy m_domPasteAccessPolicy { DOMPasteAccessPolicy::NotRequestedYet };
GestureScope m_scope { GestureScope::All };
MonotonicTime m_startTime { MonotonicTime::now() };
@@ -123,7 +123,7 @@
public:
WEBCORE_EXPORT static RefPtr<UserGestureToken> currentUserGesture();
- WEBCORE_EXPORT static bool processingUserGesture();
+ WEBCORE_EXPORT static bool processingUserGesture(Document* = nullptr);
WEBCORE_EXPORT static bool processingUserGestureForMedia();
// If a document is provided, its last known user gesture timestamp is updated.
