Commit 5b50b674 authored by alexmos's avatar alexmos Committed by Commit bot

Add URL validation to navigations initiated via RenderFrameProxyHosts.

When we changed RenderFrameProxyHost::OnOpenURL to use the transfer
logic in r377832, we lost FilterURL validation provided by
RenderFrameHostImpl::OpenURL.  This CL adds that validation to
navigations initiated via proxies.

BUG=595339
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1812723002

Cr-Commit-Position: refs/heads/master@{#381788}
parent 6d63e3c4
...@@ -248,6 +248,9 @@ void RenderFrameProxyHost::OnDetach() { ...@@ -248,6 +248,9 @@ void RenderFrameProxyHost::OnDetach() {
void RenderFrameProxyHost::OnOpenURL( void RenderFrameProxyHost::OnOpenURL(
const FrameHostMsg_OpenURL_Params& params) { const FrameHostMsg_OpenURL_Params& params) {
GURL validated_url(params.url);
GetProcess()->FilterURL(false, &validated_url);
// Verify that we are in the same BrowsingInstance as the current // Verify that we are in the same BrowsingInstance as the current
// RenderFrameHost. // RenderFrameHost.
RenderFrameHostImpl* current_rfh = frame_tree_node_->current_frame_host(); RenderFrameHostImpl* current_rfh = frame_tree_node_->current_frame_host();
...@@ -261,7 +264,7 @@ void RenderFrameProxyHost::OnOpenURL( ...@@ -261,7 +264,7 @@ void RenderFrameProxyHost::OnOpenURL(
// TODO(alexmos, creis): Figure out whether |params.user_gesture| needs to be // TODO(alexmos, creis): Figure out whether |params.user_gesture| needs to be
// passed in as well. // passed in as well.
frame_tree_node_->navigator()->RequestTransferURL( frame_tree_node_->navigator()->RequestTransferURL(
current_rfh, params.url, site_instance_.get(), std::vector<GURL>(), current_rfh, validated_url, site_instance_.get(), std::vector<GURL>(),
params.referrer, ui::PAGE_TRANSITION_LINK, GlobalRequestID(), params.referrer, ui::PAGE_TRANSITION_LINK, GlobalRequestID(),
params.should_replace_current_entry); params.should_replace_current_entry);
} }
......
...@@ -42,6 +42,7 @@ ...@@ -42,6 +42,7 @@
#include "content/public/browser/resource_dispatcher_host.h" #include "content/public/browser/resource_dispatcher_host.h"
#include "content/public/common/browser_side_navigation_policy.h" #include "content/public/common/browser_side_navigation_policy.h"
#include "content/public/common/content_switches.h" #include "content/public/common/content_switches.h"
#include "content/public/common/url_constants.h"
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test_utils.h" #include "content/public/test/content_browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h" #include "content/public/test/test_navigation_observer.h"
...@@ -3696,6 +3697,42 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, OpenPopupWithRemoteParent) { ...@@ -3696,6 +3697,42 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, OpenPopupWithRemoteParent) {
EXPECT_TRUE(success); EXPECT_TRUE(success);
} }
// Test that cross-process popups can't be navigated to disallowed URLs by
// their opener. This ensures that proper URL validation is performed when
// RenderFrameProxyHosts are navigated. See https://crbug.com/595339.
IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, NavigatePopupToIllegalURL) {
GURL main_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
EXPECT_TRUE(NavigateToURL(shell(), main_url));
// Open a cross-site popup.
GURL popup_url(embedded_test_server()->GetURL("b.com", "/title2.html"));
Shell* popup = OpenPopup(shell()->web_contents(), popup_url, "foo");
EXPECT_TRUE(popup);
EXPECT_NE(popup->web_contents()->GetSiteInstance(),
shell()->web_contents()->GetSiteInstance());
// From the opener, navigate the popup to a file:/// URL. This should be
// disallowed and result in an about:blank navigation.
GURL file_url("file:///");
NavigateNamedFrame(shell()->web_contents(), file_url, "foo");
EXPECT_TRUE(WaitForLoadStop(popup->web_contents()));
EXPECT_EQ(GURL(url::kAboutBlankURL),
popup->web_contents()->GetLastCommittedURL());
// Navigate popup back to a cross-site URL.
EXPECT_TRUE(NavigateToURL(popup, popup_url));
EXPECT_NE(popup->web_contents()->GetSiteInstance(),
shell()->web_contents()->GetSiteInstance());
// Now try the same test with a chrome:// URL.
GURL chrome_url(std::string(kChromeUIScheme) + "://" +
std::string(kChromeUIGpuHost));
NavigateNamedFrame(shell()->web_contents(), chrome_url, "foo");
EXPECT_TRUE(WaitForLoadStop(popup->web_contents()));
EXPECT_EQ(GURL(url::kAboutBlankURL),
popup->web_contents()->GetLastCommittedURL());
}
// Verify that named frames are discoverable from their opener's ancestors. // Verify that named frames are discoverable from their opener's ancestors.
// See https://crbug.com/511474. // See https://crbug.com/511474.
IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment