Commit 9e2f21d8 authored by Aaron Colwell's avatar Aaron Colwell Committed by Commit Bot

Fix AppCacheHost crash caused by blob:null/xxxx style URLs.

This fixes renderer kills caused by creating an iframe whose src is
a blob URL created by an opaque origin.

Bug: 1010120
Change-Id: I16b5f148522aa390d2d01c2e08353e834c6a48fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1836533Reviewed-by: default avatarMarijn Kruisselbrink <mek@chromium.org>
Reviewed-by: default avatarAaron Colwell <acolwell@chromium.org>
Commit-Queue: Aaron Colwell <acolwell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702234}
parent f77be189
...@@ -64,6 +64,8 @@ bool CanAccessDocumentURL(int process_id, const GURL& document_url) { ...@@ -64,6 +64,8 @@ bool CanAccessDocumentURL(int process_id, const GURL& document_url) {
document_url.IsAboutSrcdoc() || // <iframe srcdoc= ...> case. document_url.IsAboutSrcdoc() || // <iframe srcdoc= ...> case.
document_url.IsAboutBlank() || // <iframe src="javascript:''"> case. document_url.IsAboutBlank() || // <iframe src="javascript:''"> case.
document_url == GURL("data:,") || // CSP blocked_urls. document_url == GURL("data:,") || // CSP blocked_urls.
(document_url.SchemeIsBlob() && // <iframe src="blob:null/xx"> case.
url::Origin::Create(document_url).opaque()) ||
security_policy->CanAccessDataForOrigin(process_id, security_policy->CanAccessDataForOrigin(process_id,
document_url) || document_url) ||
!security_policy->HasSecurityState(process_id); // process shutdown. !security_policy->HasSecurityState(process_id); // process shutdown.
......
...@@ -141,6 +141,7 @@ class AppCacheHostTest : public testing::Test { ...@@ -141,6 +141,7 @@ class AppCacheHostTest : public testing::Test {
int GetInUseCount(const url::Origin& origin) { return inuse_[origin]; } int GetInUseCount(const url::Origin& origin) { return inuse_[origin]; }
bool is_empty() const { return inuse_.empty(); }
void reset() { inuse_.clear(); } void reset() { inuse_.clear(); }
// Map from origin to count of inuse notifications. // Map from origin to count of inuse notifications.
...@@ -210,6 +211,18 @@ TEST_F(AppCacheHostTest, Basic) { ...@@ -210,6 +211,18 @@ TEST_F(AppCacheHostTest, Basic) {
} }
TEST_F(AppCacheHostTest, SelectNoCache) { TEST_F(AppCacheHostTest, SelectNoCache) {
// Lock process to |kProcessLockURL| so we can only accept URLs from
// that site.
const GURL kProcessLockURL("http://whatever/");
ChildProcessSecurityPolicyImpl::GetInstance()->LockToOrigin(
IsolationContext(&browser_context_), kProcessIdForTest, kProcessLockURL);
const std::vector<GURL> kDocumentURLs = {
GURL("http://whatever/"),
GURL("blob:http://whatever/6f7dc725-2131-4f8b-85ed-4f43d175324e"),
GURL("about:blank"), GURL("about:srcdoc"),
GURL("blob:null/6f7dc725-2131-4f8b-85ed-4f43d175324e")};
for (const GURL& document_url : kDocumentURLs) {
scoped_refptr<MockQuotaManagerProxy> mock_quota_proxy = scoped_refptr<MockQuotaManagerProxy> mock_quota_proxy =
base::MakeRefCounted<MockQuotaManagerProxy>(); base::MakeRefCounted<MockQuotaManagerProxy>();
service_.set_quota_manager_proxy(mock_quota_proxy.get()); service_.set_quota_manager_proxy(mock_quota_proxy.get());
...@@ -219,18 +232,31 @@ TEST_F(AppCacheHostTest, SelectNoCache) { ...@@ -219,18 +232,31 @@ TEST_F(AppCacheHostTest, SelectNoCache) {
mock_frontend_.last_status_ = mock_frontend_.last_status_ =
blink::mojom::AppCacheStatus::APPCACHE_STATUS_OBSOLETE; blink::mojom::AppCacheStatus::APPCACHE_STATUS_OBSOLETE;
const GURL kDocAndOriginUrl(GURL("http://whatever/").GetOrigin()); const url::Origin kOrigin(url::Origin::Create(document_url));
const url::Origin kOrigin(url::Origin::Create(kDocAndOriginUrl));
{ {
AppCacheHost host(kHostIdForTest, kProcessIdForTest, kRenderFrameIdForTest, AppCacheHost host(kHostIdForTest, kProcessIdForTest,
mojo::NullRemote(), &service_); kRenderFrameIdForTest, mojo::NullRemote(), &service_);
host.set_frontend_for_testing(&mock_frontend_); host.set_frontend_for_testing(&mock_frontend_);
host.SelectCache(kDocAndOriginUrl, blink::mojom::kAppCacheNoCacheId,
{
mojo::test::BadMessageObserver bad_message_observer;
host.SelectCache(document_url, blink::mojom::kAppCacheNoCacheId,
GURL()); GURL());
EXPECT_EQ(1, mock_quota_proxy->GetInUseCount(kOrigin));
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(bad_message_observer.got_bad_message());
}
if (kOrigin.opaque()) {
EXPECT_TRUE(mock_quota_proxy->is_empty());
} else {
EXPECT_EQ(1, mock_quota_proxy->GetInUseCount(kOrigin))
<< " document_url " << document_url;
}
// We should have received an OnCacheSelected msg // We should have received an OnCacheSelected msg
EXPECT_EQ(blink::mojom::kAppCacheNoCacheId, mock_frontend_.last_cache_id_); EXPECT_EQ(blink::mojom::kAppCacheNoCacheId,
mock_frontend_.last_cache_id_);
EXPECT_EQ(blink::mojom::AppCacheStatus::APPCACHE_STATUS_UNCACHED, EXPECT_EQ(blink::mojom::AppCacheStatus::APPCACHE_STATUS_UNCACHED,
mock_frontend_.last_status_); mock_frontend_.last_status_);
...@@ -243,6 +269,7 @@ TEST_F(AppCacheHostTest, SelectNoCache) { ...@@ -243,6 +269,7 @@ TEST_F(AppCacheHostTest, SelectNoCache) {
} }
EXPECT_EQ(0, mock_quota_proxy->GetInUseCount(kOrigin)); EXPECT_EQ(0, mock_quota_proxy->GetInUseCount(kOrigin));
service_.set_quota_manager_proxy(nullptr); service_.set_quota_manager_proxy(nullptr);
}
} }
TEST_F(AppCacheHostTest, ForeignEntry) { TEST_F(AppCacheHostTest, ForeignEntry) {
...@@ -477,9 +504,9 @@ TEST_F(AppCacheHostTest, SelectCacheAllowed) { ...@@ -477,9 +504,9 @@ TEST_F(AppCacheHostTest, SelectCacheAllowed) {
mock_frontend_.content_blocked_ = false; mock_frontend_.content_blocked_ = false;
mock_frontend_.appcache_accessed_ = false; mock_frontend_.appcache_accessed_ = false;
const GURL kDocAndOriginUrl(GURL("http://whatever/").GetOrigin()); const GURL kDocAndOriginUrl("http://whatever/");
const url::Origin kOrigin(url::Origin::Create(kDocAndOriginUrl)); const url::Origin kOrigin(url::Origin::Create(kDocAndOriginUrl));
const GURL kManifestUrl(GURL("http://whatever/cache.manifest")); const GURL kManifestUrl("http://whatever/cache.manifest");
{ {
AppCacheHost host(kHostIdForTest, kProcessIdForTest, kRenderFrameIdForTest, AppCacheHost host(kHostIdForTest, kProcessIdForTest, kRenderFrameIdForTest,
mojo::NullRemote(), &service_); mojo::NullRemote(), &service_);
...@@ -648,7 +675,20 @@ TEST_F(AppCacheHostTest, SelectCacheURLsForWrongSite) { ...@@ -648,7 +675,20 @@ TEST_F(AppCacheHostTest, SelectCacheURLsForWrongSite) {
bad_message_observer.WaitForBadMessage()); bad_message_observer.WaitForBadMessage());
} }
// Verify that a document URL from the wrong site triggers a bad message. // Verify that a document URL with an inner hostname from the wrong site
// triggers a bad message.
{
const GURL kDocumentURL = kProcessLockURL;
mojo::test::BadMessageObserver bad_message_observer;
host_remote->SelectCache(
kDocumentURL, blink::mojom::kAppCacheNoCacheId,
GURL("blob:http://whatever/6f7dc725-2131-4f8b-85ed-4f43d175324e"));
EXPECT_EQ("ACH_SELECT_CACHE_MANIFEST_URL_ACCESS_NOT_ALLOWED",
bad_message_observer.WaitForBadMessage());
}
// Verify that a manifest URL from the wrong site triggers a bad message.
{ {
const GURL kDocumentURL = kProcessLockURL; const GURL kDocumentURL = kProcessLockURL;
const GURL kManifestURL("http://whatever/"); const GURL kManifestURL("http://whatever/");
......
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