Commit 3caf3b93 authored by Michael van Ouwerkerk's avatar Michael van Ouwerkerk Committed by Commit Bot

Check domain instead of host for remote copy.

Bug: 1049055
Change-Id: Ic4d0ed82c68a3a9105a50b112f8472a4f8272392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036106Reviewed-by: default avatarRichard Knoll <knollr@chromium.org>
Commit-Queue: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738555}
parent 1f2f2dfe
...@@ -38,7 +38,6 @@ ...@@ -38,7 +38,6 @@
#include "ui/message_center/public/cpp/notification.h" #include "ui/message_center/public/cpp/notification.h"
#include "ui/message_center/public/cpp/notification_types.h" #include "ui/message_center/public/cpp/notification_types.h"
#include "ui/message_center/public/cpp/notifier_id.h" #include "ui/message_center/public/cpp/notifier_id.h"
#include "url/origin.h"
namespace { namespace {
constexpr size_t kMaxImageDownloadSize = 5 * 1024 * 1024; constexpr size_t kMaxImageDownloadSize = 5 * 1024 * 1024;
...@@ -164,7 +163,7 @@ void RemoteCopyMessageHandler::HandleImage(const std::string& image_url) { ...@@ -164,7 +163,7 @@ void RemoteCopyMessageHandler::HandleImage(const std::string& image_url) {
return; return;
} }
if (!IsOriginAllowed(url)) { if (!IsImageSourceAllowed(url)) {
Finish(RemoteCopyHandleMessageResult::kFailureImageOriginNotAllowed); Finish(RemoteCopyHandleMessageResult::kFailureImageOriginNotAllowed);
return; return;
} }
...@@ -187,15 +186,19 @@ void RemoteCopyMessageHandler::HandleImage(const std::string& image_url) { ...@@ -187,15 +186,19 @@ void RemoteCopyMessageHandler::HandleImage(const std::string& image_url) {
kMaxImageDownloadSize); kMaxImageDownloadSize);
} }
bool RemoteCopyMessageHandler::IsOriginAllowed(const GURL& image_url) { bool RemoteCopyMessageHandler::IsImageSourceAllowed(const GURL& image_url) {
url::Origin image_origin = url::Origin::Create(image_url);
std::vector<std::string> parts = std::vector<std::string> parts =
base::SplitString(kRemoteCopyAllowedOrigins.Get(), ",", base::SplitString(kRemoteCopyAllowedOrigins.Get(), ",",
base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY); base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
for (const auto& part : parts) { for (const auto& part : parts) {
url::Origin allowed_origin = url::Origin::Create(GURL(part)); GURL allowed_origin(part);
if (image_origin.IsSameOriginWith(allowed_origin)) // The actual image URL may have a hash in the subdomain. This means we
// cannot match the entire host - we'll match the domain instead.
if (image_url.SchemeIs(allowed_origin.scheme_piece()) &&
image_url.DomainIs(allowed_origin.host_piece()) &&
image_url.EffectiveIntPort() == allowed_origin.EffectiveIntPort()) {
return true; return true;
}
} }
return false; return false;
} }
......
...@@ -38,7 +38,7 @@ class RemoteCopyMessageHandler : public SharingMessageHandler, ...@@ -38,7 +38,7 @@ class RemoteCopyMessageHandler : public SharingMessageHandler,
void OnImageDecoded(const SkBitmap& decoded_image) override; void OnImageDecoded(const SkBitmap& decoded_image) override;
void OnDecodeImageFailed() override; void OnDecodeImageFailed() override;
bool IsOriginAllowed(const GURL& image_url); bool IsImageSourceAllowed(const GURL& image_url);
private: private:
void HandleText(const std::string& text); void HandleText(const std::string& text);
......
...@@ -47,12 +47,12 @@ class RemoteCopyMessageHandlerTest : public SharedClipboardTestBase { ...@@ -47,12 +47,12 @@ class RemoteCopyMessageHandlerTest : public SharedClipboardTestBase {
return message; return message;
} }
bool IsOriginAllowed(const std::string& image_url, bool IsImageSourceAllowed(const std::string& image_url,
const std::string& param_value) { const std::string& param_value) {
base::test::ScopedFeatureList feature_list; base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeatureWithParameters( feature_list.InitAndEnableFeatureWithParameters(
kRemoteCopyReceiver, {{kRemoteCopyAllowedOrigins.name, param_value}}); kRemoteCopyReceiver, {{kRemoteCopyAllowedOrigins.name, param_value}});
return message_handler_->IsOriginAllowed(GURL(image_url)); return message_handler_->IsImageSourceAllowed(GURL(image_url));
} }
protected: protected:
...@@ -90,15 +90,21 @@ TEST_F(RemoteCopyMessageHandlerTest, NotificationWithDeviceName) { ...@@ -90,15 +90,21 @@ TEST_F(RemoteCopyMessageHandlerTest, NotificationWithDeviceName) {
kHistogramName, RemoteCopyHandleMessageResult::kSuccessHandledText, 1); kHistogramName, RemoteCopyHandleMessageResult::kSuccessHandledText, 1);
} }
TEST_F(RemoteCopyMessageHandlerTest, IsOriginAllowed) { TEST_F(RemoteCopyMessageHandlerTest, IsImageSourceAllowed) {
std::string image_url = "https://foo.com/image.png"; std::string image_url = "https://foo.com/image.png";
std::string image_url_with_subdomain = "https://www.foo.com/image.png"; std::string image_url_with_subdomain = "https://www.foo.com/image.png";
std::string image_origin = "https://foo.com"; EXPECT_TRUE(IsImageSourceAllowed(image_url, "https://foo.com"));
EXPECT_TRUE(IsOriginAllowed(image_url, image_origin)); EXPECT_TRUE(
EXPECT_FALSE(IsOriginAllowed(image_url_with_subdomain, image_origin)); IsImageSourceAllowed(image_url_with_subdomain, "https://foo.com"));
EXPECT_FALSE(IsOriginAllowed(image_url, "")); EXPECT_FALSE(IsImageSourceAllowed(image_url, ""));
EXPECT_FALSE(IsOriginAllowed(image_url, "foo][#';/.,")); EXPECT_FALSE(IsImageSourceAllowed(image_url, "foo][#';/.,"));
EXPECT_FALSE(IsOriginAllowed(image_url, "https://bar.com")); EXPECT_FALSE(IsImageSourceAllowed(image_url, "https://bar.com"));
EXPECT_TRUE(IsOriginAllowed(image_url, image_origin + ",https://bar.com")); EXPECT_FALSE(IsImageSourceAllowed(image_url,
EXPECT_TRUE(IsOriginAllowed(image_url, "https://bar.com," + image_origin)); "https://foo.com:80")); // not default port
EXPECT_TRUE(
IsImageSourceAllowed(image_url, "https://foo.com:443")); // default port
EXPECT_TRUE(
IsImageSourceAllowed(image_url, "https://foo.com,https://bar.com"));
EXPECT_TRUE(
IsImageSourceAllowed(image_url, "https://bar.com,https://foo.com"));
} }
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