Commit 4124592f authored by mattm's avatar mattm Committed by Commit bot

Safebrowsing download protection: handle data URIs

BUG=440572

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

Cr-Commit-Position: refs/heads/master@{#308157}
parent 2246f354
...@@ -37,6 +37,7 @@ ...@@ -37,6 +37,7 @@
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/download_item.h" #include "content/public/browser/download_item.h"
#include "content/public/browser/page_navigator.h" #include "content/public/browser/page_navigator.h"
#include "crypto/sha2.h"
#include "google_apis/google_api_keys.h" #include "google_apis/google_api_keys.h"
#include "net/base/escape.h" #include "net/base/escape.h"
#include "net/base/load_flags.h" #include "net/base/load_flags.h"
...@@ -138,6 +139,22 @@ enum SBStatsType { ...@@ -138,6 +139,22 @@ enum SBStatsType {
// ALWAYS ADD NEW VALUES BEFORE THIS ONE. // ALWAYS ADD NEW VALUES BEFORE THIS ONE.
DOWNLOAD_CHECKS_MAX DOWNLOAD_CHECKS_MAX
}; };
// Prepares URLs to be put into a ping message. Currently this just shortens
// data: URIs, other URLs are included verbatim.
std::string SanitizeUrl(const GURL& url) {
std::string spec = url.spec();
if (url.SchemeIs(url::kDataScheme)) {
size_t comma_pos = spec.find(',');
if (comma_pos != std::string::npos && comma_pos != spec.size() - 1) {
std::string hash_value = crypto::SHA256HashString(spec);
spec.erase(comma_pos + 1);
spec += base::HexEncode(hash_value.data(), hash_value.size());
}
}
return spec;
}
} // namespace } // namespace
// Parent SafeBrowsing::Client class used to lookup the bad binary // Parent SafeBrowsing::Client class used to lookup the bad binary
...@@ -467,7 +484,8 @@ class DownloadProtectionService::CheckClientDownloadRequest ...@@ -467,7 +484,8 @@ class DownloadProtectionService::CheckClientDownloadRequest
*reason = REASON_INVALID_URL; *reason = REASON_INVALID_URL;
return false; return false;
} }
if ((!final_url.IsStandard() && !final_url.SchemeIsBlob()) || if ((!final_url.IsStandard() && !final_url.SchemeIsBlob() &&
!final_url.SchemeIs(url::kDataScheme)) ||
final_url.SchemeIsFile()) { final_url.SchemeIsFile()) {
*reason = REASON_UNSUPPORTED_URL_SCHEME; *reason = REASON_UNSUPPORTED_URL_SCHEME;
return false; return false;
...@@ -697,16 +715,16 @@ class DownloadProtectionService::CheckClientDownloadRequest ...@@ -697,16 +715,16 @@ class DownloadProtectionService::CheckClientDownloadRequest
return; return;
ClientDownloadRequest request; ClientDownloadRequest request;
request.set_url(item_->GetUrlChain().back().spec()); request.set_url(SanitizeUrl(item_->GetUrlChain().back()));
request.mutable_digests()->set_sha256(item_->GetHash()); request.mutable_digests()->set_sha256(item_->GetHash());
request.set_length(item_->GetReceivedBytes()); request.set_length(item_->GetReceivedBytes());
for (size_t i = 0; i < item_->GetUrlChain().size(); ++i) { for (size_t i = 0; i < item_->GetUrlChain().size(); ++i) {
ClientDownloadRequest::Resource* resource = request.add_resources(); ClientDownloadRequest::Resource* resource = request.add_resources();
resource->set_url(item_->GetUrlChain()[i].spec()); resource->set_url(SanitizeUrl(item_->GetUrlChain()[i]));
if (i == item_->GetUrlChain().size() - 1) { if (i == item_->GetUrlChain().size() - 1) {
// The last URL in the chain is the download URL. // The last URL in the chain is the download URL.
resource->set_type(ClientDownloadRequest::DOWNLOAD_URL); resource->set_type(ClientDownloadRequest::DOWNLOAD_URL);
resource->set_referrer(item_->GetReferrerUrl().spec()); resource->set_referrer(SanitizeUrl(item_->GetReferrerUrl()));
DVLOG(2) << "dl url " << resource->url(); DVLOG(2) << "dl url " << resource->url();
if (!item_->GetRemoteAddress().empty()) { if (!item_->GetRemoteAddress().empty()) {
resource->set_remote_ip(item_->GetRemoteAddress()); resource->set_remote_ip(item_->GetRemoteAddress());
...@@ -723,16 +741,16 @@ class DownloadProtectionService::CheckClientDownloadRequest ...@@ -723,16 +741,16 @@ class DownloadProtectionService::CheckClientDownloadRequest
for (size_t i = 0; i < tab_redirects_.size(); ++i) { for (size_t i = 0; i < tab_redirects_.size(); ++i) {
ClientDownloadRequest::Resource* resource = request.add_resources(); ClientDownloadRequest::Resource* resource = request.add_resources();
DVLOG(2) << "tab redirect " << i << " " << tab_redirects_[i].spec(); DVLOG(2) << "tab redirect " << i << " " << tab_redirects_[i].spec();
resource->set_url(tab_redirects_[i].spec()); resource->set_url(SanitizeUrl(tab_redirects_[i]));
resource->set_type(ClientDownloadRequest::TAB_REDIRECT); resource->set_type(ClientDownloadRequest::TAB_REDIRECT);
} }
if (tab_url_.is_valid()) { if (tab_url_.is_valid()) {
ClientDownloadRequest::Resource* resource = request.add_resources(); ClientDownloadRequest::Resource* resource = request.add_resources();
resource->set_url(tab_url_.spec()); resource->set_url(SanitizeUrl(tab_url_));
DVLOG(2) << "tab url " << resource->url(); DVLOG(2) << "tab url " << resource->url();
resource->set_type(ClientDownloadRequest::TAB_URL); resource->set_type(ClientDownloadRequest::TAB_URL);
if (tab_referrer_url_.is_valid()) { if (tab_referrer_url_.is_valid()) {
resource->set_referrer(tab_referrer_url_.spec()); resource->set_referrer(SanitizeUrl(tab_referrer_url_));
DVLOG(2) << "tab referrer " << resource->referrer(); DVLOG(2) << "tab referrer " << resource->referrer();
} }
} }
......
...@@ -174,6 +174,8 @@ class DownloadProtectionService { ...@@ -174,6 +174,8 @@ class DownloadProtectionService {
CheckClientDownloadHTTPS); CheckClientDownloadHTTPS);
FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest, FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest,
CheckClientDownloadBlob); CheckClientDownloadBlob);
FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest,
CheckClientDownloadData);
FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest, FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest,
CheckClientDownloadZip); CheckClientDownloadZip);
FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest, FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest,
......
...@@ -306,6 +306,10 @@ class DownloadProtectionServiceTest : public testing::Test { ...@@ -306,6 +306,10 @@ class DownloadProtectionServiceTest : public testing::Test {
return certs.empty() ? NULL : certs[0]; return certs.empty() ? NULL : certs[0];
} }
const ClientDownloadRequest* GetClientDownloadRequest() const {
return last_client_download_request_.get();
}
bool HasClientDownloadRequest() const { bool HasClientDownloadRequest() const {
return last_client_download_request_.get() != NULL; return last_client_download_request_.get() != NULL;
} }
...@@ -928,6 +932,89 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadBlob) { ...@@ -928,6 +932,89 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadBlob) {
#endif #endif
} }
TEST_F(DownloadProtectionServiceTest, CheckClientDownloadData) {
ClientDownloadResponse response;
response.set_verdict(ClientDownloadResponse::DANGEROUS);
net::FakeURLFetcherFactory factory(NULL);
factory.SetFakeResponse(DownloadProtectionService::GetDownloadRequestUrl(),
response.SerializeAsString(), net::HTTP_OK,
net::URLRequestStatus::SUCCESS);
base::FilePath a_tmp(FILE_PATH_LITERAL("a.tmp"));
base::FilePath a_exe(FILE_PATH_LITERAL("a.exe"));
std::vector<GURL> url_chain;
url_chain.push_back(
GURL("data:text/html:base64,"));
url_chain.push_back(
GURL("data:text/html:base64,blahblahblah"));
url_chain.push_back(
GURL("data:application/octet-stream:base64,blahblah"));
GURL referrer("data:text/html:base64,foobar");
std::string hash = "hash";
content::MockDownloadItem item;
EXPECT_CALL(item, GetFullPath()).WillRepeatedly(ReturnRef(a_tmp));
EXPECT_CALL(item, GetTargetFilePath()).WillRepeatedly(ReturnRef(a_exe));
EXPECT_CALL(item, GetUrlChain()).WillRepeatedly(ReturnRef(url_chain));
EXPECT_CALL(item, GetReferrerUrl()).WillRepeatedly(ReturnRef(referrer));
EXPECT_CALL(item, GetTabUrl()).WillRepeatedly(ReturnRef(GURL::EmptyGURL()));
EXPECT_CALL(item, GetTabReferrerUrl())
.WillRepeatedly(ReturnRef(GURL::EmptyGURL()));
EXPECT_CALL(item, GetHash()).WillRepeatedly(ReturnRef(hash));
EXPECT_CALL(item, GetReceivedBytes()).WillRepeatedly(Return(100));
EXPECT_CALL(item, HasUserGesture()).WillRepeatedly(Return(true));
EXPECT_CALL(item, GetRemoteAddress()).WillRepeatedly(Return(""));
EXPECT_CALL(*sb_service_->mock_database_manager(),
MatchDownloadWhitelistUrl(_)).WillRepeatedly(Return(false));
EXPECT_CALL(*binary_feature_extractor_.get(), CheckSignature(a_tmp, _))
.Times(1);
EXPECT_CALL(*binary_feature_extractor_.get(), ExtractImageHeaders(a_tmp, _))
.Times(1);
download_service_->CheckClientDownload(
&item,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
base::Unretained(this)));
MessageLoop::current()->Run();
#if defined(OS_WIN)
EXPECT_TRUE(IsResult(DownloadProtectionService::DANGEROUS));
#else
EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
#endif
#if defined(OS_WIN) || defined(OS_MACOSX)
// OSX sends pings for evaluation purposes.
ASSERT_TRUE(HasClientDownloadRequest());
const ClientDownloadRequest& request = *GetClientDownloadRequest();
const char kExpectedUrl[] =
"data:application/octet-stream:base64,"
"ACBF6DFC6F907662F566CA0241DFE8690C48661F440BA1BBD0B86C582845CCC8";
const char kExpectedRedirect1[] = "data:text/html:base64,";
const char kExpectedRedirect2[] =
"data:text/html:base64,"
"620680767E15717A57DB11D94D1BEBD32B3344EBC5994DF4FB07B0D473F4EF6B";
const char kExpectedReferrer[] =
"data:text/html:base64,"
"06E2C655B9F7130B508FFF86FD19B57E6BF1A1CFEFD6EFE1C3EB09FE24EF456A";
EXPECT_EQ(hash, request.digests().sha256());
EXPECT_EQ(kExpectedUrl, request.url());
EXPECT_EQ(3, request.resources_size());
EXPECT_TRUE(RequestContainsResource(request,
ClientDownloadRequest::DOWNLOAD_REDIRECT,
kExpectedRedirect1, ""));
EXPECT_TRUE(RequestContainsResource(request,
ClientDownloadRequest::DOWNLOAD_REDIRECT,
kExpectedRedirect2, ""));
EXPECT_TRUE(RequestContainsResource(request,
ClientDownloadRequest::DOWNLOAD_URL,
kExpectedUrl, kExpectedReferrer));
ClearClientDownloadRequest();
#else
EXPECT_FALSE(HasClientDownloadRequest());
#endif
}
TEST_F(DownloadProtectionServiceTest, CheckClientDownloadZip) { TEST_F(DownloadProtectionServiceTest, CheckClientDownloadZip) {
ClientDownloadResponse response; ClientDownloadResponse response;
response.set_verdict(ClientDownloadResponse::SAFE); response.set_verdict(ClientDownloadResponse::SAFE);
......
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