postMessage to site-isolated iframe should have correct event.origin

https://bugs.webkit.org/show_bug.cgi?id=265494 rdar://118907443 Reviewed by Pascoe. In LocalDOMWindow::postMessageFromRemoteFrame we were passing the destination document as the source document, and we were using that document to get the source origin, which made event.origin the origin of the receiver instead of sender. To fix it, pass the sourceOrigin along from the sending process to the receiving process. The source document was also used in a call to InspectorInstrumentation::consoleAgentEnabled, and instead of the source document we'll use the receiving LocalDOMWindow's document, which is probably more correct but either way it is just a perf optimization to not get the stack trace if the web inspector is not open. * LayoutTests/http/tests/site-isolation/post-message-expected.txt: * LayoutTests/http/tests/site-isolation/post-message.html: * Source/WebCore/page/LocalDOMWindow.cpp: (WebCore::LocalDOMWindow::processPostMessage): (WebCore::LocalDOMWindow::postMessage): (WebCore::LocalDOMWindow::postMessageFromRemoteFrame): * Source/WebCore/page/LocalDOMWindow.h: * Source/WebCore/page/RemoteDOMWindow.cpp: (WebCore::RemoteDOMWindow::postMessage): * Source/WebCore/page/RemoteFrameClient.h: * Source/WebKit/UIProcess/WebProcessProxy.cpp: (WebKit::WebProcessProxy::postMessageToRemote): * Source/WebKit/UIProcess/WebProcessProxy.h: * Source/WebKit/UIProcess/WebProcessProxy.messages.in: * Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp: (WebKit::WebRemoteFrameClient::postMessageToRemote): * Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.h: * Source/WebKit/WebProcess/WebProcess.cpp: (WebKit::WebProcess::remotePostMessage): * Source/WebKit/WebProcess/WebProcess.h: * Source/WebKit/WebProcess/WebProcess.messages.in: Canonical link: https://commits.webkit.org/271281@main
WebKit · Nov 29, 2023 · bd7fed3 · bd7fed3
1 parent f120e42
commit bd7fed3
Show file tree

Hide file tree

Showing 14 changed files with 32 additions and 31 deletions.
diff --git a/LayoutTests/http/tests/site-isolation/post-message-expected.txt b/LayoutTests/http/tests/site-isolation/post-message-expected.txt
@@ -5,6 +5,6 @@ PASS posting a non-serializable message threw an exception: DataCloneError: The
 PASS successfullyParsed is true
 
 TEST COMPLETE
-PASS received 'iframe received hello'
-PASS received 'iframe received world'
+PASS received 'iframe received hello' from http://localhost:8000
+PASS received 'iframe received world' from http://localhost:8000
 
diff --git a/LayoutTests/http/tests/site-isolation/post-message.html b/LayoutTests/http/tests/site-isolation/post-message.html
@@ -8,7 +8,7 @@
 
 let messageCount = 0;
 addEventListener("message", (event) => {
-    testPassed("received '" + event.data + "'");
+    testPassed("received '" + event.data + "' from " + event.origin);
     messageCount = messageCount + 1;
     if (messageCount == 2) { testRunner.notifyDone() }
 });

diff --git a/Source/WebCore/page/LocalDOMWindow.cpp b/Source/WebCore/page/LocalDOMWindow.cpp
@@ -911,21 +911,18 @@ ExceptionOr<Storage*> LocalDOMWindow::localStorage()
     return m_localStorage.get();
 }
 
-void LocalDOMWindow::processPostMessage(JSC::JSGlobalObject& lexicalGlobalObject, RefPtr<Document>&& sourceDocument, const MessageWithMessagePorts& message, RefPtr<WindowProxy>&& incumbentWindowProxy, RefPtr<SecurityOrigin>&& targetOrigin)
+void LocalDOMWindow::processPostMessage(JSC::JSGlobalObject& lexicalGlobalObject, const String& sourceOrigin, const MessageWithMessagePorts& message, RefPtr<WindowProxy>&& incumbentWindowProxy, RefPtr<SecurityOrigin>&& targetOrigin)
 {
-    // Capture the source of the message.  We need to do this synchronously
-    // in order to capture the source of the message correctly.
-    auto sourceOrigin = sourceDocument->securityOrigin().toString();
     // Capture stack trace only when inspector front-end is loaded as it may be time consuming.
     RefPtr<ScriptCallStack> stackTrace;
-    if (InspectorInstrumentation::consoleAgentEnabled(sourceDocument.get()))
+    if (InspectorInstrumentation::consoleAgentEnabled(document()))
         stackTrace = createScriptCallStack(JSExecState::currentState());
 
     auto postMessageIdentifier = InspectorInstrumentation::willPostMessage(*frame());
 
     auto userGestureToForward = UserGestureIndicator::currentUserGesture();
 
-    protectedDocument()->checkedEventLoop()->queueTask(TaskSource::PostedMessageQueue, [this, protectedThis = Ref { *this }, message = message, incumbentWindowProxy = WTFMove(incumbentWindowProxy), sourceOrigin = WTFMove(sourceOrigin), userGestureToForward = WTFMove(userGestureToForward), postMessageIdentifier, stackTrace = WTFMove(stackTrace), targetOrigin = WTFMove(targetOrigin)]() mutable {
+    protectedDocument()->checkedEventLoop()->queueTask(TaskSource::PostedMessageQueue, [this, protectedThis = Ref { *this }, message = message, incumbentWindowProxy = WTFMove(incumbentWindowProxy), sourceOrigin, userGestureToForward = WTFMove(userGestureToForward), postMessageIdentifier, stackTrace = WTFMove(stackTrace), targetOrigin = WTFMove(targetOrigin)]() mutable {
         if (!isCurrentlyDisplayedInFrame())
             return;
 
@@ -994,19 +991,19 @@ ExceptionOr<void> LocalDOMWindow::postMessage(JSC::JSGlobalObject& lexicalGlobal
     if (disentangledPorts.hasException())
         return disentangledPorts.releaseException();
 
+    // Capture the source of the message. We need to do this synchronously
+    // in order to capture the source of the message correctly.
+    auto sourceOrigin = sourceDocument->securityOrigin().toString();
+
     // Schedule the message.
     RefPtr incumbentWindowProxy = incumbentWindow.frame() ? &incumbentWindow.frame()->windowProxy() : nullptr;
     MessageWithMessagePorts message { messageData.releaseReturnValue(), disentangledPorts.releaseReturnValue() };
-    processPostMessage(lexicalGlobalObject, WTFMove(sourceDocument), message, WTFMove(incumbentWindowProxy), targetSecurityOrigin.releaseReturnValue());
+    processPostMessage(lexicalGlobalObject, sourceOrigin, message, WTFMove(incumbentWindowProxy), targetSecurityOrigin.releaseReturnValue());
     return { };
 }
 
-void LocalDOMWindow::postMessageFromRemoteFrame(JSC::JSGlobalObject& lexicalGlobalObject, std::optional<WebCore::SecurityOriginData> target, const WebCore::MessageWithMessagePorts& message)
+void LocalDOMWindow::postMessageFromRemoteFrame(JSC::JSGlobalObject& lexicalGlobalObject, const String& sourceOrigin, std::optional<WebCore::SecurityOriginData> target, const WebCore::MessageWithMessagePorts& message)
 {
-    RefPtr sourceDocument = document();
-    if (!sourceDocument)
-        return;
-
     if (!frame())
         return;
 
@@ -1016,7 +1013,7 @@ void LocalDOMWindow::postMessageFromRemoteFrame(JSC::JSGlobalObject& lexicalGlob
     if (target)
         targetOrigin = target->securityOrigin();
 
-    processPostMessage(lexicalGlobalObject, WTFMove(sourceDocument), message, WTFMove(incumbentWindowProxy), WTFMove(targetOrigin));
+    processPostMessage(lexicalGlobalObject, sourceOrigin, message, WTFMove(incumbentWindowProxy), WTFMove(targetOrigin));
 }
 
 DOMSelection* LocalDOMWindow::getSelection()

diff --git a/Source/WebCore/page/LocalDOMWindow.h b/Source/WebCore/page/LocalDOMWindow.h
@@ -269,7 +269,7 @@ class LocalDOMWindow final
     {
         return postMessage(globalObject, incumbentWindow, message, WindowPostMessageOptions { WTFMove(targetOrigin), WTFMove(transfer) });
     }
-    WEBCORE_EXPORT void postMessageFromRemoteFrame(JSC::JSGlobalObject&, std::optional<WebCore::SecurityOriginData> target, const WebCore::MessageWithMessagePorts&);
+    WEBCORE_EXPORT void postMessageFromRemoteFrame(JSC::JSGlobalObject&, const String& sourceOrigin, std::optional<WebCore::SecurityOriginData> target, const WebCore::MessageWithMessagePorts&);
 
     void languagesChanged();
 
@@ -443,7 +443,7 @@ class LocalDOMWindow final
     void decrementGamepadEventListenerCount();
 #endif
 
-    void processPostMessage(JSC::JSGlobalObject&, RefPtr<Document>&&, const MessageWithMessagePorts&, RefPtr<WindowProxy>&&, RefPtr<SecurityOrigin>&&);
+    void processPostMessage(JSC::JSGlobalObject&, const String& origin, const MessageWithMessagePorts&, RefPtr<WindowProxy>&&, RefPtr<SecurityOrigin>&&);
     bool m_shouldPrintWhenFinishedLoading { false };
     bool m_suspendedForDocumentSuspension { false };
     bool m_isSuspendingObservers { false };

diff --git a/Source/WebCore/page/RemoteDOMWindow.cpp b/Source/WebCore/page/RemoteDOMWindow.cpp
@@ -153,9 +153,13 @@ ExceptionOr<void> RemoteDOMWindow::postMessage(JSC::JSGlobalObject& lexicalGloba
     if (disentangledPorts.hasException())
         return messageData.releaseException();
 
+    // Capture the source of the message. We need to do this synchronously
+    // in order to capture the source of the message correctly.
+    auto sourceOrigin = sourceDocument->securityOrigin().toString();
+
     MessageWithMessagePorts messageWithPorts { messageData.releaseReturnValue(), disentangledPorts.releaseReturnValue() };
     if (auto* remoteFrame = frame())
-        remoteFrame->client().postMessageToRemote(remoteFrame->frameID(), target, messageWithPorts);
+        remoteFrame->client().postMessageToRemote(remoteFrame->frameID(), sourceOrigin, target, messageWithPorts);
     return { };
 }
 

diff --git a/Source/WebCore/page/RemoteFrameClient.h b/Source/WebCore/page/RemoteFrameClient.h
@@ -42,7 +42,7 @@ class RemoteFrameClient {
 public:
     virtual void frameDetached() = 0;
     virtual void sizeDidChange(IntSize) = 0;
-    virtual void postMessageToRemote(FrameIdentifier, std::optional<SecurityOriginData>, const MessageWithMessagePorts&) = 0;
+    virtual void postMessageToRemote(FrameIdentifier, const String& sourceOrigin, std::optional<SecurityOriginData>, const MessageWithMessagePorts&) = 0;
     virtual void changeLocation(FrameLoadRequest&&) = 0;
     virtual String renderTreeAsText(size_t baseIndent, OptionSet<RenderAsTextFlag>) = 0;
     virtual void broadcastFrameRemovalToOtherProcesses() = 0;

diff --git a/Source/WebKit/UIProcess/WebProcessProxy.cpp b/Source/WebKit/UIProcess/WebProcessProxy.cpp
@@ -1386,10 +1386,10 @@ void WebProcessProxy::didDestroyUserGestureToken(uint64_t identifier)
         m_userInitiatedActionByAuthorizationTokenMap.remove(*removed->authorizationToken());
 }
 
-void WebProcessProxy::postMessageToRemote(WebCore::FrameIdentifier identifier, std::optional<WebCore::SecurityOriginData> target, const WebCore::MessageWithMessagePorts& message)
+void WebProcessProxy::postMessageToRemote(WebCore::FrameIdentifier identifier, const String& sourceOrigin, std::optional<WebCore::SecurityOriginData> target, const WebCore::MessageWithMessagePorts& message)
 {
     if (RefPtr destinationFrame = WebFrameProxy::webFrame(identifier))
-        destinationFrame->protectedProcess()->send(Messages::WebProcess::RemotePostMessage(identifier, target, message), 0);
+        destinationFrame->protectedProcess()->send(Messages::WebProcess::RemotePostMessage(identifier, sourceOrigin, target, message), 0);
 }
 
 void WebProcessProxy::closeRemoteFrame(WebCore::FrameIdentifier frameID)

diff --git a/Source/WebKit/UIProcess/WebProcessProxy.h b/Source/WebKit/UIProcess/WebProcessProxy.h
@@ -540,7 +540,7 @@ class WebProcessProxy : public AuxiliaryProcessProxy {
     void updateBackForwardItem(const BackForwardListItemState&);
     void didDestroyFrame(WebCore::FrameIdentifier, WebPageProxyIdentifier);
     void didDestroyUserGestureToken(uint64_t);
-    void postMessageToRemote(WebCore::FrameIdentifier, std::optional<WebCore::SecurityOriginData>, const WebCore::MessageWithMessagePorts&);
+    void postMessageToRemote(WebCore::FrameIdentifier, const String& sourceOrigin, std::optional<WebCore::SecurityOriginData>, const WebCore::MessageWithMessagePorts&);
     void closeRemoteFrame(WebCore::FrameIdentifier);
     void focusRemoteFrame(WebCore::FrameIdentifier);
     void renderTreeAsText(WebCore::FrameIdentifier, size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag>, CompletionHandler<void(String&&)>&&);

diff --git a/Source/WebKit/UIProcess/WebProcessProxy.messages.in b/Source/WebKit/UIProcess/WebProcessProxy.messages.in
@@ -88,7 +88,7 @@ messages -> WebProcessProxy LegacyReceiver {
     SetAppBadge(std::optional<WebKit::WebPageProxyIdentifier> pageIdentifier, WebCore::SecurityOriginData origin, std::optional<uint64_t> badge)
     SetClientBadge(WebKit::WebPageProxyIdentifier pageIdentifier, WebCore::SecurityOriginData origin, std::optional<uint64_t> badge)
 
-    PostMessageToRemote(WebCore::FrameIdentifier identifier, std::optional<WebCore::SecurityOriginData> target, struct WebCore::MessageWithMessagePorts message)
+    PostMessageToRemote(WebCore::FrameIdentifier identifier, String sourceOrigin, std::optional<WebCore::SecurityOriginData> target, struct WebCore::MessageWithMessagePorts message)
     CloseRemoteFrame(WebCore::FrameIdentifier identifier)
     FocusRemoteFrame(WebCore::FrameIdentifier identifier)
 

diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.cpp
@@ -65,9 +65,9 @@ void WebRemoteFrameClient::sizeDidChange(WebCore::IntSize size)
     m_frame->updateRemoteFrameSize(size);
 }
 
-void WebRemoteFrameClient::postMessageToRemote(WebCore::FrameIdentifier identifier, std::optional<WebCore::SecurityOriginData> target, const WebCore::MessageWithMessagePorts& message)
+void WebRemoteFrameClient::postMessageToRemote(WebCore::FrameIdentifier identifier, const String& sourceOrigin, std::optional<WebCore::SecurityOriginData> target, const WebCore::MessageWithMessagePorts& message)
 {
-    WebProcess::singleton().send(Messages::WebProcessProxy::PostMessageToRemote(identifier, target, message), 0);
+    WebProcess::singleton().send(Messages::WebProcessProxy::PostMessageToRemote(identifier, sourceOrigin, target, message), 0);
 }
 
 void WebRemoteFrameClient::changeLocation(WebCore::FrameLoadRequest&& request)

diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.h b/Source/WebKit/WebProcess/WebCoreSupport/WebRemoteFrameClient.h
@@ -45,7 +45,7 @@ class WebRemoteFrameClient final : public WebCore::RemoteFrameClient, public Web
 private:
     void frameDetached() final;
     void sizeDidChange(WebCore::IntSize) final;
-    void postMessageToRemote(WebCore::FrameIdentifier, std::optional<WebCore::SecurityOriginData>, const WebCore::MessageWithMessagePorts&) final;
+    void postMessageToRemote(WebCore::FrameIdentifier, const String& sourceOrigin, std::optional<WebCore::SecurityOriginData>, const WebCore::MessageWithMessagePorts&) final;
     void changeLocation(WebCore::FrameLoadRequest&&) final;
     String renderTreeAsText(size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag>) final;
     void broadcastFrameRemovalToOtherProcesses() final;

diff --git a/Source/WebKit/WebProcess/WebProcess.cpp b/Source/WebKit/WebProcess/WebProcess.cpp
@@ -1364,7 +1364,7 @@ void WebProcess::setEnhancedAccessibility(bool flag)
     WebCore::AXObjectCache::setEnhancedUserInterfaceAccessibility(flag);
 }
 
-void WebProcess::remotePostMessage(WebCore::FrameIdentifier identifier, std::optional<WebCore::SecurityOriginData> target, const WebCore::MessageWithMessagePorts& message)
+void WebProcess::remotePostMessage(WebCore::FrameIdentifier identifier, const String& sourceOrigin, std::optional<WebCore::SecurityOriginData> target, const WebCore::MessageWithMessagePorts& message)
 {
     RefPtr webFrame = WebProcess::singleton().webFrame(identifier);
     if (!webFrame)
@@ -1386,7 +1386,7 @@ void WebProcess::remotePostMessage(WebCore::FrameIdentifier identifier, std::opt
     if (!globalObject)
         return;
 
-    domWindow->postMessageFromRemoteFrame(*globalObject, target, message);
+    domWindow->postMessageFromRemoteFrame(*globalObject, sourceOrigin, target, message);
 }
 
 void WebProcess::renderTreeAsText(WebCore::FrameIdentifier frameIdentifier, size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag> behavior, CompletionHandler<void(String&&)>&& completionHandler)

diff --git a/Source/WebKit/WebProcess/WebProcess.h b/Source/WebKit/WebProcess/WebProcess.h
@@ -495,7 +495,7 @@ class WebProcess : public AuxiliaryProcess
     void platformSetCacheModel(CacheModel);
 
     void setEnhancedAccessibility(bool);
-    void remotePostMessage(WebCore::FrameIdentifier, std::optional<WebCore::SecurityOriginData>, const WebCore::MessageWithMessagePorts&);
+    void remotePostMessage(WebCore::FrameIdentifier, const String& sourceOrigin, std::optional<WebCore::SecurityOriginData>, const WebCore::MessageWithMessagePorts&);
 
     void renderTreeAsText(WebCore::FrameIdentifier, size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag>, CompletionHandler<void(String&&)>&&);
 

diff --git a/Source/WebKit/WebProcess/WebProcess.messages.in b/Source/WebKit/WebProcess/WebProcess.messages.in
@@ -214,7 +214,7 @@ messages -> WebProcess LegacyReceiver NotRefCounted {
 
     ReleaseMemory() -> ()
 
-    RemotePostMessage(WebCore::FrameIdentifier frameIdentifier, std::optional<WebCore::SecurityOriginData> target, struct WebCore::MessageWithMessagePorts message)
+    RemotePostMessage(WebCore::FrameIdentifier frameIdentifier, String sourceOrigin, std::optional<WebCore::SecurityOriginData> target, struct WebCore::MessageWithMessagePorts message)
 
     RenderTreeAsText(WebCore::FrameIdentifier frameIdentifier, size_t baseIndent, OptionSet<WebCore::RenderAsTextFlag> behavior) -> (String renderTreeDump) Synchronous