Check image url against allowlist from feature param.

Bug: 1018138
Change-Id: Ia6ede078dc04cc80fcc21450b22e0513c6da249f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1919139
Commit-Queue: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Reviewed-by: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715707}

chrome/browser/sharing/shared_clipboard/feature_flags.cc

@@ -14,5 +14,8 @@ const base::Feature kSharedClipboardUI{"SharedClipboardUI",
     defined(OS_CHROMEOS)
 const base::Feature kRemoteCopyReceiver{"RemoteCopyReceiver",
                                         base::FEATURE_DISABLED_BY_DEFAULT};
+
+const base::FeatureParam<std::string> kRemoteCopyAllowedOrigins = {
+    &kRemoteCopyReceiver, "RemoteCopyAllowedOrigins", ""};
 #endif  // defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_LINUX) ||
         // defined(OS_CHROMEOS)

chrome/browser/sharing/shared_clipboard/feature_flags.h

@@ -5,7 +5,10 @@
 #ifndef CHROME_BROWSER_SHARING_SHARED_CLIPBOARD_FEATURE_FLAGS_H_
 #define CHROME_BROWSER_SHARING_SHARED_CLIPBOARD_FEATURE_FLAGS_H_
 
+#include <string>
+
 #include "base/feature_list.h"
+#include "base/metrics/field_trial_params.h"
 #include "build/build_config.h"
 
 // Feature to allow devices to receive the shared clipboard message.
@@ -18,6 +21,9 @@ extern const base::Feature kSharedClipboardUI;
     defined(OS_CHROMEOS)
 // Feature to enable handling remote copy messages.
 extern const base::Feature kRemoteCopyReceiver;
+
+// List of allowed origins to fetch images from, comma separated.
+extern const base::FeatureParam<std::string> kRemoteCopyAllowedOrigins;
 #endif  // defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_LINUX) ||
         // defined(OS_CHROMEOS)
 

chrome/browser/sharing/shared_clipboard/remote_copy_browsertest.cc

@@ -66,14 +66,6 @@ class RemoteCopyBrowserTestBase : public InProcessBrowserTest {
     ui::Clipboard::DestroyClipboardForCurrentThread();
   }
 
-  GURL StartServerAndGetURL(const std::string& relative_url) {
-    server_ = std::make_unique<net::EmbeddedTestServer>(
-        net::EmbeddedTestServer::TYPE_HTTP);
-    server_->ServeFilesFromSourceDirectory(GetChromeTestDataDir());
-    EXPECT_TRUE(server_->Start());
-    return server_->GetURL(relative_url);
-  }
-
   gcm::IncomingMessage CreateMessage(const std::string& device_name,
                                      base::Optional<std::string> text,
                                      base::Optional<GURL> image_url) {
@@ -174,7 +166,15 @@ IN_PROC_BROWSER_TEST_F(RemoteCopyDisabledBrowserTest, FeatureDisabled) {
 class RemoteCopyBrowserTest : public RemoteCopyBrowserTestBase {
  public:
   RemoteCopyBrowserTest() {
-    feature_list_.InitAndEnableFeature(kRemoteCopyReceiver);
+    server_ = std::make_unique<net::EmbeddedTestServer>(
+        net::EmbeddedTestServer::TYPE_HTTP);
+    server_->ServeFilesFromSourceDirectory(GetChromeTestDataDir());
+    EXPECT_TRUE(server_->Start());
+
+    url::Origin allowlist_origin = url::Origin::Create(server_->base_url());
+    feature_list_.InitAndEnableFeatureWithParameters(
+        kRemoteCopyReceiver,
+        {{kRemoteCopyAllowedOrigins.name, allowlist_origin.Serialize()}});
   }
 };
 
@@ -201,8 +201,7 @@ IN_PROC_BROWSER_TEST_F(RemoteCopyBrowserTest, ImageUrl) {
   ASSERT_TRUE(GetAvailableClipboardTypes().empty());
 
   // Send a message with an image url.
-  SendImageMessage(kDeviceName,
-                   StartServerAndGetURL("/image_decoding/droids.jpg"));
+  SendImageMessage(kDeviceName, server_->GetURL("/image_decoding/droids.jpg"));
 
   // The image is in the clipboard and a notification is shown.
   std::vector<base::string16> types = GetAvailableClipboardTypes();
@@ -232,8 +231,7 @@ IN_PROC_BROWSER_TEST_F(RemoteCopyBrowserTest, TextThenImageUrl) {
   ASSERT_EQ(kText, ReadClipboardText());
 
   // Send a message with an image url.
-  SendImageMessage(kDeviceName,
-                   StartServerAndGetURL("/image_decoding/droids.jpg"));
+  SendImageMessage(kDeviceName, server_->GetURL("/image_decoding/droids.jpg"));
 
   // The image is in the clipboard and the text has been cleared.
   types = GetAvailableClipboardTypes();

chrome/browser/sharing/shared_clipboard/remote_copy_message_handler.cc

@@ -4,15 +4,17 @@
 
 #include "chrome/browser/sharing/shared_clipboard/remote_copy_message_handler.h"
 
-#include <memory>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/guid.h"
 #include "base/strings/string16.h"
+#include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/notifications/notification_display_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sharing/shared_clipboard/feature_flags.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/sync/protocol/sharing_message.pb.h"
 #include "components/sync/protocol/sharing_remote_copy_message.pb.h"
@@ -29,7 +31,7 @@
 #include "ui/message_center/public/cpp/notification.h"
 #include "ui/message_center/public/cpp/notification_types.h"
 #include "ui/message_center/public/cpp/notifier_id.h"
-#include "url/gurl.h"
+#include "url/origin.h"
 
 namespace {
 const net::NetworkTrafficAnnotationTag kTrafficAnnotation =
@@ -100,12 +102,17 @@ void RemoteCopyMessageHandler::HandleText(const std::string& text) {
 
 void RemoteCopyMessageHandler::HandleImage(const std::string& image_url) {
   GURL url(image_url);
-  // TODO(mvanouwerkerk): Whitelist check.
+
   if (!network::IsUrlPotentiallyTrustworthy(url)) {
     Finish();
     return;
   }
 
+  if (!IsOriginAllowed(url)) {
+    Finish();
+    return;
+  }
+
   auto request = std::make_unique<network::ResourceRequest>();
   request->url = url;
   // This request should be unauthenticated (no cookies), and shouldn't be
@@ -124,6 +131,19 @@ void RemoteCopyMessageHandler::HandleImage(const std::string& image_url) {
       network::SimpleURLLoader::kMaxBoundedStringDownloadSize);
 }
 
+bool RemoteCopyMessageHandler::IsOriginAllowed(const GURL& image_url) {
+  url::Origin image_origin = url::Origin::Create(image_url);
+  std::vector<std::string> parts =
+      base::SplitString(kRemoteCopyAllowedOrigins.Get(), ",",
+                        base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  for (const auto& part : parts) {
+    url::Origin allowed_origin = url::Origin::Create(GURL(part));
+    if (image_origin.IsSameOriginWith(allowed_origin))
+      return true;
+  }
+  return false;
+}
+
 void RemoteCopyMessageHandler::OnURLLoadComplete(
     std::unique_ptr<std::string> content) {
   url_loader_.reset();

chrome/browser/sharing/shared_clipboard/remote_copy_message_handler.h

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "chrome/browser/image_decoder.h"
 #include "chrome/browser/sharing/sharing_message_handler.h"
+#include "url/gurl.h"
 
 class Profile;
 
@@ -33,6 +34,8 @@ class RemoteCopyMessageHandler : public SharingMessageHandler,
   void OnImageDecoded(const SkBitmap& decoded_image) override;
   void OnDecodeImageFailed() override;
 
+  bool IsOriginAllowed(const GURL& image_url);
+
  private:
   void HandleText(const std::string& text);
   void HandleImage(const std::string& image_url);

chrome/browser/sharing/shared_clipboard/remote_copy_message_handler_unittest.cc

@@ -7,6 +7,8 @@
 #include "base/bind_helpers.h"
 #include "base/guid.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/sharing/shared_clipboard/feature_flags.h"
 #include "chrome/browser/sharing/shared_clipboard/shared_clipboard_test_base.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/testing_profile.h"
@@ -42,6 +44,14 @@ class RemoteCopyMessageHandlerTest : public SharedClipboardTestBase {
     return message;
   }
 
+  bool IsOriginAllowed(const std::string& image_url,
+                       const std::string& param_value) {
+    base::test::ScopedFeatureList feature_list;
+    feature_list.InitAndEnableFeatureWithParameters(
+        kRemoteCopyReceiver, {{kRemoteCopyAllowedOrigins.name, param_value}});
+    return message_handler_->IsOriginAllowed(GURL(image_url));
+  }
+
  protected:
   std::unique_ptr<RemoteCopyMessageHandler> message_handler_;
 
@@ -71,3 +81,16 @@ TEST_F(RemoteCopyMessageHandlerTest, NotificationWithDeviceName) {
                 base::ASCIIToUTF16(kDeviceNameInMessage)),
             GetNotification().title());
 }
+
+TEST_F(RemoteCopyMessageHandlerTest, IsOriginAllowed) {
+  std::string image_url = "https://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(IsOriginAllowed(image_url, image_origin));
+  EXPECT_FALSE(IsOriginAllowed(image_url_with_subdomain, image_origin));
+  EXPECT_FALSE(IsOriginAllowed(image_url, ""));
+  EXPECT_FALSE(IsOriginAllowed(image_url, "foo][#';/.,"));
+  EXPECT_FALSE(IsOriginAllowed(image_url, "https://bar.com"));
+  EXPECT_TRUE(IsOriginAllowed(image_url, image_origin + ",https://bar.com"));
+  EXPECT_TRUE(IsOriginAllowed(image_url, "https://bar.com," + image_origin));
+}