Commit 0be4d34d authored by nasko's avatar nasko Committed by Commit bot

Add a check for mismatching URL and origin in the renderer process.

The renderer process kills with reason RFH_INVALID_ORIGIN_ON_COMMIT are
checking parameters passed from the renderer process. However, we are
lacking a bunch of information since it is in the browser process and
it is hard to reason about what the state was in the renderer.

We can also try to implement similar checking in the renderer process,
so we can catch these mismatches closer to where we have more data.
This CL implements some of the checks performed browser side, but
it is a set that should catch the majority of the cases we have seen
in crash reports.

BUG=628677
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2151323003
Cr-Commit-Position: refs/heads/master@{#406145}
parent d04d3ef3
...@@ -41,6 +41,7 @@ ...@@ -41,6 +41,7 @@
#include "content/public/common/content_switches.h" #include "content/public/common/content_switches.h"
#include "content/public/common/page_state.h" #include "content/public/common/page_state.h"
#include "content/public/common/url_constants.h" #include "content/public/common/url_constants.h"
#include "content/public/common/web_preferences.h"
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h" #include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h" #include "content/public/test/content_browser_test_utils.h"
...@@ -2802,4 +2803,36 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostManagerTest, ...@@ -2802,4 +2803,36 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostManagerTest,
EXPECT_EQ(title, entry->GetTitle()); EXPECT_EQ(title, entry->GetTitle());
} }
// Ensure that document hosted on file: URL can successfully execute pushState
// with arbitrary origin, when universal access setting is enabled.
// TODO(nasko): The test is disabled on Mac, since universal access from file
// scheme behaves differently.
#if defined(OS_MACOSX)
#define MAYBE_EnsureUniversalAccessFromFileSchemeSucceeds \
DISABLED_EnsureUniversalAccessFromFileSchemeSucceeds
#else
#define MAYBE_EnsureUniversalAccessFromFileSchemeSucceeds \
EnsureUniversalAccessFromFileSchemeSucceeds
#endif
IN_PROC_BROWSER_TEST_F(RenderFrameHostManagerTest,
MAYBE_EnsureUniversalAccessFromFileSchemeSucceeds) {
StartEmbeddedServer();
WebContentsImpl* web_contents =
static_cast<WebContentsImpl*>(shell()->web_contents());
FrameTreeNode* root = web_contents->GetFrameTree()->root();
WebPreferences prefs =
web_contents->GetRenderViewHost()->GetWebkitPreferences();
prefs.allow_universal_access_from_file_urls = true;
web_contents->GetRenderViewHost()->UpdateWebkitPreferences(prefs);
GURL file_url = GetTestUrl("", "title1.html");
ASSERT_TRUE(NavigateToURL(shell(), file_url));
EXPECT_EQ(1, web_contents->GetController().GetEntryCount());
EXPECT_TRUE(ExecuteScript(
root, "window.history.pushState({}, '', 'https://chromium.org');"));
EXPECT_EQ(2, web_contents->GetController().GetEntryCount());
EXPECT_TRUE(web_contents->GetMainFrame()->IsRenderFrameLive());
}
} // namespace content } // namespace content
...@@ -4773,6 +4773,20 @@ void RenderFrameImpl::SendDidCommitProvisionalLoad( ...@@ -4773,6 +4773,20 @@ void RenderFrameImpl::SendDidCommitProvisionalLoad(
render_view_->SetZoomLevel(render_view_->page_zoom_level()); render_view_->SetZoomLevel(render_view_->page_zoom_level());
} }
// Standard URLs must match the reported origin, when it is not unique.
// This check is very similar to RenderFrameHostImpl::CanCommitOrigin, but
// adapted to the renderer process side.
if (!params.origin.unique() && params.url.IsStandard() &&
render_view_->GetWebkitPreferences().web_security_enabled) {
// Exclude file: URLs when settings allow them access any origin.
if (params.origin.scheme() != url::kFileScheme ||
!render_view_->GetWebkitPreferences()
.allow_universal_access_from_file_urls) {
CHECK(params.origin.IsSameOriginWith(url::Origin(params.url)))
<< " url:" << params.url << " origin:" << params.origin;
}
}
// This message needs to be sent before any of allowScripts(), // This message needs to be sent before any of allowScripts(),
// allowImages(), allowPlugins() is called for the new page, so that when // allowImages(), allowPlugins() is called for the new page, so that when
// these functions send a ViewHostMsg_ContentBlocked message, it arrives // these functions send a ViewHostMsg_ContentBlocked message, it arrives
......
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