Skip to content

Commit

Permalink
Use about:blank as the failback URL if the filter denies a navigation.
Browse files Browse the repository at this point in the history 
BUG=117417
Review URL: https://chromiumcodereview.appspot.com/9794009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128057 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
cevans@chromium.org committed Mar 21, 2012
1 parent c34059c commit 6b7cd8f
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 27 deletions.
4 changes: 2 additions & 2 deletions chrome/browser/geolocation/chrome_geolocation_permission_context_unittest.cc
View file
Expand Up @@ -411,8 +411,8 @@ TEST_F(GeolocationPermissionContextTests, CancelGeolocationPermissionRequest) {
}

TEST_F(GeolocationPermissionContextTests, InvalidURL) {
GURL invalid_embedder;
GURL requesting_frame("about:blank");
GURL invalid_embedder("about:blank");
GURL requesting_frame;
NavigateAndCommit(invalid_embedder);
EXPECT_EQ(0U, infobar_tab_helper()->infobar_count());
RequestGeolocationPermission(
Expand Down
44 changes: 27 additions & 17 deletions content/browser/renderer_host/render_view_host_impl.cc
View file
Expand Up @@ -495,7 +495,7 @@ void RenderViewHostImpl::DragTargetDragEnter(
// The URL could have been cobbled together from any highlighted text string,
// and can't be interpreted as a capability.
WebDropData filtered_data(drop_data);
FilterURL(policy, renderer_id, &filtered_data.url);
FilterURL(policy, renderer_id, false, &filtered_data.url);

// The filenames vector, on the other hand, does represent a capability to
// access the given files.
Expand Down Expand Up @@ -1039,15 +1039,15 @@ void RenderViewHostImpl::OnMsgNavigate(const IPC::Message& msg) {
// renderer to load the URL and grant the renderer the privileges to request
// the URL. To prevent this attack, we block the renderer from inserting
// banned URLs into the navigation controller in the first place.
FilterURL(policy, renderer_id, &validated_params.url);
FilterURL(policy, renderer_id, &validated_params.referrer.url);
FilterURL(policy, renderer_id, false, &validated_params.url);
FilterURL(policy, renderer_id, true, &validated_params.referrer.url);
for (std::vector<GURL>::iterator it(validated_params.redirects.begin());
it != validated_params.redirects.end(); ++it) {
FilterURL(policy, renderer_id, &(*it));
FilterURL(policy, renderer_id, false, &(*it));
}
FilterURL(policy, renderer_id, &validated_params.searchable_form_url);
FilterURL(policy, renderer_id, &validated_params.password_form.origin);
FilterURL(policy, renderer_id, &validated_params.password_form.action);
FilterURL(policy, renderer_id, true, &validated_params.searchable_form_url);
FilterURL(policy, renderer_id, true, &validated_params.password_form.origin);
FilterURL(policy, renderer_id, true, &validated_params.password_form.action);

delegate_->DidNavigate(this, validated_params);
}
Expand Down Expand Up @@ -1139,10 +1139,10 @@ void RenderViewHostImpl::OnMsgContextMenu(

// We don't validate |unfiltered_link_url| so that this field can be used
// when users want to copy the original link URL.
FilterURL(policy, renderer_id, &validated_params.link_url);
FilterURL(policy, renderer_id, &validated_params.src_url);
FilterURL(policy, renderer_id, &validated_params.page_url);
FilterURL(policy, renderer_id, &validated_params.frame_url);
FilterURL(policy, renderer_id, true, &validated_params.link_url);
FilterURL(policy, renderer_id, true, &validated_params.src_url);
FilterURL(policy, renderer_id, false, &validated_params.page_url);
FilterURL(policy, renderer_id, true, &validated_params.frame_url);

view->ShowContextMenu(validated_params);
}
Expand All @@ -1159,7 +1159,7 @@ void RenderViewHostImpl::OnMsgOpenURL(const GURL& url,
int64 source_frame_id) {
GURL validated_url(url);
FilterURL(ChildProcessSecurityPolicyImpl::GetInstance(),
GetProcess()->GetID(), &validated_url);
GetProcess()->GetID(), false, &validated_url);

delegate_->RequestOpenURL(
validated_url, referrer, disposition, source_frame_id);
Expand Down Expand Up @@ -1244,8 +1244,8 @@ void RenderViewHostImpl::OnMsgStartDragging(

// Allow drag of Javascript URLs to enable bookmarklet drag to bookmark bar.
if (!filtered_data.url.SchemeIs(chrome::kJavaScriptScheme))
FilterURL(policy, GetProcess()->GetID(), &filtered_data.url);
FilterURL(policy, GetProcess()->GetID(), &filtered_data.html_base_url);
FilterURL(policy, GetProcess()->GetID(), false, &filtered_data.url);
FilterURL(policy, GetProcess()->GetID(), false, &filtered_data.html_base_url);
view->StartDragging(filtered_data, drag_operations_mask, image, image_offset);
}

Expand Down Expand Up @@ -1448,9 +1448,19 @@ void RenderViewHostImpl::ToggleSpeechInput() {

void RenderViewHostImpl::FilterURL(ChildProcessSecurityPolicyImpl* policy,
int renderer_id,
bool empty_allowed,
GURL* url) {
if (!url->is_valid())
return; // We don't need to block invalid URLs.
if (empty_allowed && url->is_empty())
return;

if (!url->is_valid()) {
// Have to use about:blank for the denied case, instead of an empty GURL.
// This is because the browser treats navigation to an empty GURL as a
// navigation to the home page. This is often a privileged page
// (chrome://newtab/) which is exactly what we don't want.
*url = GURL(chrome::kAboutBlankURL);
return;
}

if (url->SchemeIs(chrome::kAboutScheme)) {
// The renderer treats all URLs in the about: scheme as being about:blank.
Expand All @@ -1463,7 +1473,7 @@ void RenderViewHostImpl::FilterURL(ChildProcessSecurityPolicyImpl* policy,
// URL. This prevents us from storing the blocked URL and becoming confused
// later.
VLOG(1) << "Blocked URL " << url->spec();
*url = GURL();
*url = GURL(chrome::kAboutBlankURL);
}
}

Expand Down
6 changes: 4 additions & 2 deletions content/browser/renderer_host/render_view_host_impl.h
View file
Expand Up @@ -390,10 +390,12 @@ class CONTENT_EXPORT RenderViewHostImpl
return is_waiting_for_unload_ack_;
}

// Checks that the given renderer can request |url|, if not it sets it to an
// empty url.
// Checks that the given renderer can request |url|, if not it sets it to
// about:blank.
// empty_allowed must be set to false for navigations for security reasons.
static void FilterURL(ChildProcessSecurityPolicyImpl* policy,
int renderer_id,
bool empty_allowed,
GURL* url);

// NOTE: Do not add functions that just send an IPC message that are called in
Expand Down
4 changes: 2 additions & 2 deletions content/browser/renderer_host/render_view_host_unittest.cc
View file
Expand Up @@ -133,8 +133,8 @@ TEST_F(RenderViewHostTest, StartDragging) {
drop_data.url = file_url;
drop_data.html_base_url = file_url;
test_rvh()->TestOnMsgStartDragging(drop_data);
EXPECT_TRUE(view_delegate.drag_url().is_empty());
EXPECT_TRUE(view_delegate.html_base_url().is_empty());
EXPECT_EQ(GURL("about:blank"), view_delegate.drag_url());
EXPECT_EQ(GURL("about:blank"), view_delegate.html_base_url());

GURL http_url = GURL("http://www.domain.com/index.html");
drop_data.url = http_url;
Expand Down
35 changes: 31 additions & 4 deletions content/browser/tab_contents/tab_contents.cc
View file
Expand Up @@ -1435,10 +1435,17 @@ void TabContents::OnDidStartProvisionalLoadForFrame(int64 frame_id,
const GURL& url) {
bool is_error_page = (url.spec() == chrome::kUnreachableWebDataURL);
GURL validated_url(url);
GURL validated_opener_url(opener_url);
GetRenderViewHostImpl()->FilterURL(
ChildProcessSecurityPolicyImpl::GetInstance(),
GetRenderProcessHost()->GetID(),
false,
&validated_url);
GetRenderViewHostImpl()->FilterURL(
ChildProcessSecurityPolicyImpl::GetInstance(),
GetRenderProcessHost()->GetID(),
true,
&validated_opener_url);

RenderViewHost* rvh =
render_manager_.pending_render_view_host() ?
Expand All @@ -1451,7 +1458,8 @@ void TabContents::OnDidStartProvisionalLoadForFrame(int64 frame_id,
if (is_main_frame) {
// Notify observers about the provisional change in the main frame URL.
FOR_EACH_OBSERVER(WebContentsObserver, observers_,
ProvisionalChangeToMainFrameUrl(url, opener_url));
ProvisionalChangeToMainFrameUrl(validated_url,
validated_opener_url));
}
}

Expand All @@ -1462,18 +1470,36 @@ void TabContents::OnDidRedirectProvisionalLoad(int32 page_id,
// TODO(creis): Remove this method and have the pre-rendering code listen to
// the ResourceDispatcherHost's RESOURCE_RECEIVED_REDIRECT notification
// instead. See http://crbug.com/78512.
GURL validated_source_url(source_url);
GURL validated_target_url(target_url);
GURL validated_opener_url(opener_url);
GetRenderViewHostImpl()->FilterURL(
ChildProcessSecurityPolicyImpl::GetInstance(),
GetRenderProcessHost()->GetID(),
false,
&validated_source_url);
GetRenderViewHostImpl()->FilterURL(
ChildProcessSecurityPolicyImpl::GetInstance(),
GetRenderProcessHost()->GetID(),
false,
&validated_target_url);
GetRenderViewHostImpl()->FilterURL(
ChildProcessSecurityPolicyImpl::GetInstance(),
GetRenderProcessHost()->GetID(),
true,
&validated_opener_url);
NavigationEntry* entry;
if (page_id == -1)
entry = controller_.GetPendingEntry();
else
entry = controller_.GetEntryWithPageID(GetSiteInstance(), page_id);
if (!entry || entry->GetURL() != source_url)
if (!entry || entry->GetURL() != validated_source_url)
return;

// Notify observers about the provisional change in the main frame URL.
FOR_EACH_OBSERVER(WebContentsObserver, observers_,
ProvisionalChangeToMainFrameUrl(target_url,
opener_url));
ProvisionalChangeToMainFrameUrl(validated_target_url,
validated_opener_url));
}

void TabContents::OnDidFailProvisionalLoadWithError(
Expand All @@ -1489,6 +1515,7 @@ void TabContents::OnDidFailProvisionalLoadWithError(
GetRenderViewHostImpl()->FilterURL(
ChildProcessSecurityPolicyImpl::GetInstance(),
GetRenderProcessHost()->GetID(),
false,
&validated_url);

if (net::ERR_ABORTED == params.error_code) {
Expand Down

0 comments on commit 6b7cd8f

Please sign in to comment.