From c774694b8679cd4c57a61f45566cc6280e6e99ee Mon Sep 17 00:00:00 2001 From: Roger Tawa Date: Thu, 16 Mar 2023 20:44:39 +0000 Subject: [PATCH] Disable content analysis within a tab. If the use copies or cuts data from a web page and pastes it back into the same web page, there is no need to perform content scanning since the page already has access to the data. Bug: b:256818217 Change-Id: Id1a4fdf6d4e757ff75145f256135f6d964b022e1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4219234 Commit-Queue: Roger Tawa Reviewed-by: Daniel Cheng Reviewed-by: Dominique Fauteux-Chapleau Cr-Commit-Position: refs/heads/main@{#1118326} --- .../renderer_host/clipboard_host_impl.cc | 39 +++++++++++- .../renderer_host/clipboard_host_impl.h | 4 ++ .../clipboard_host_impl_unittest.cc | 60 ++++++++++++++++++- 3 files changed, 101 insertions(+), 2 deletions(-) diff --git a/content/browser/renderer_host/clipboard_host_impl.cc b/content/browser/renderer_host/clipboard_host_impl.cc index 9bcede9fe4c860..e9734be6c41663 100644 --- a/content/browser/renderer_host/clipboard_host_impl.cc +++ b/content/browser/renderer_host/clipboard_host_impl.cc @@ -49,6 +49,26 @@ namespace content { +namespace { + +// Used to skip content analysis checks when the source and destination of the +// clipboard data are the same. +struct LastWriterInfo { + // A pointer to the last ClipboardHostImpl that committed data to the + // clipboard. + ClipboardHostImpl* writer = nullptr; + + // The sequence number of the last commit made by `writer`. + ui::ClipboardSequenceNumberToken seqno; +}; + +LastWriterInfo& GetLastWriterInfo() { + static LastWriterInfo info; + return info; +} + +} // namespace + // The amount of time that the result of a content allow request is cached // and reused for the same clipboard `seqno`. constexpr base::TimeDelta @@ -124,6 +144,7 @@ void ClipboardHostImpl::Create( } ClipboardHostImpl::~ClipboardHostImpl() { + GetLastWriterInfo() = {}; clipboard_writer_->Reset(); } @@ -494,6 +515,13 @@ void ClipboardHostImpl::WriteImage(const SkBitmap& bitmap) { void ClipboardHostImpl::CommitWrite() { clipboard_writer_ = std::make_unique( ui::ClipboardBuffer::kCopyPaste, CreateDataEndpoint()); + + // Remember the ClipboardHostImpl and associated seqno of the last write + // made to the clipboard by any ClipboardHostImpl. + GetLastWriterInfo() = { + .writer = this, + .seqno = ui::Clipboard::GetForCurrentThread()->GetSequenceNumber( + ui::ClipboardBuffer::kCopyPaste)}; } bool ClipboardHostImpl::IsRendererPasteAllowed( @@ -630,11 +658,20 @@ void ClipboardHostImpl::PerformPasteIfContentAllowed( std::string data, IsClipboardPasteContentAllowedCallback callback) { CleanupObsoleteRequests(); + + // Always allow if the source of the last clipboard commit was this host. + const LastWriterInfo& info = GetLastWriterInfo(); + if (info.writer == this && info.seqno == seqno) { + std::move(callback).Run(data); + return; + } + // Add |callback| to the callbacks associated to the sequence number, adding // an entry to the map if one does not exist. auto& request = is_allowed_requests_[seqno]; - if (request.AddCallback(std::move(callback))) + if (request.AddCallback(std::move(callback))) { StartIsPasteContentAllowedRequest(seqno, data_type, std::move(data)); + } } void ClipboardHostImpl::StartIsPasteContentAllowedRequest( diff --git a/content/browser/renderer_host/clipboard_host_impl.h b/content/browser/renderer_host/clipboard_host_impl.h index 7484a69302be2c..a761e8872f2d97 100644 --- a/content/browser/renderer_host/clipboard_host_impl.h +++ b/content/browser/renderer_host/clipboard_host_impl.h @@ -162,6 +162,10 @@ class CONTENT_EXPORT ClipboardHostImpl PerformPasteIfContentAllowed_EmptyData); FRIEND_TEST_ALL_PREFIXES(ClipboardHostImplScanTest, PerformPasteIfContentAllowed); + FRIEND_TEST_ALL_PREFIXES(ClipboardHostImplScanTest, + PerformPasteIfContentAllowed_SameHost_NotStarted); + FRIEND_TEST_ALL_PREFIXES(ClipboardHostImplScanTest, + PerformPasteIfContentAllowed_External_Started); // mojom::ClipboardHost void GetSequenceNumber(ui::ClipboardBuffer clipboard_buffer, diff --git a/content/browser/renderer_host/clipboard_host_impl_unittest.cc b/content/browser/renderer_host/clipboard_host_impl_unittest.cc index 6339b9a6da5361..c74345aa1b57e0 100644 --- a/content/browser/renderer_host/clipboard_host_impl_unittest.cc +++ b/content/browser/renderer_host/clipboard_host_impl_unittest.cc @@ -58,18 +58,26 @@ class FakeClipboardHostImpl : public ClipboardHostImpl { void StartIsPasteContentAllowedRequest( const ui::ClipboardSequenceNumberToken& seqno, const ui::ClipboardFormatType& data_type, - std::string data) override {} + std::string data) override { + ++start_count_; + } void CompleteRequest(const ui::ClipboardSequenceNumberToken& seqno, const std::string& data) { FinishPasteIfContentAllowed(seqno, data); } + size_t start_count() const { return start_count_; } + using ClipboardHostImpl::CleanupObsoleteRequests; using ClipboardHostImpl::is_paste_allowed_requests_for_testing; using ClipboardHostImpl::kIsPasteContentAllowedRequestTooOld; using ClipboardHostImpl::PasteIfPolicyAllowed; using ClipboardHostImpl::PerformPasteIfContentAllowed; + + private: + // number of times StartIsPasteContentAllowedRequest() is called. + size_t start_count_ = 0; }; class PolicyControllerTest : public ui::DataTransferPolicyController { @@ -305,6 +313,56 @@ class ClipboardHostImplScanTest : public RenderViewHostTestHarness { raw_ptr fake_clipboard_host_impl_; }; +TEST_F(ClipboardHostImplScanTest, + PerformPasteIfContentAllowed_SameHost_NotStarted) { + const std::u16string kText = u"text"; + clipboard_host_impl()->WriteText(kText); + clipboard_host_impl()->CommitWrite(); + + std::u16string read_text; + clipboard_host_impl()->ReadText( + ui::ClipboardBuffer::kCopyPaste, + base::BindLambdaForTesting( + [&read_text](const std::u16string& value) { read_text = value; })); + + // When the same document writes and then reads from the clipboard, content + // checks should be skipped. + EXPECT_EQ(0u, clipboard_host_impl()->start_count()); + EXPECT_EQ(kText, read_text); +} + +TEST_F(ClipboardHostImplScanTest, + PerformPasteIfContentAllowed_External_Started) { + const std::u16string kText = u"text"; + + // Write directly to clipboard. + { + ui::ScopedClipboardWriter writer(ui::ClipboardBuffer::kCopyPaste); + writer.WriteText(kText); + } + + std::u16string read_text; + clipboard_host_impl()->ReadText( + ui::ClipboardBuffer::kCopyPaste, + base::BindLambdaForTesting( + [&read_text](const std::u16string& value) { read_text = value; })); + + // Completing the request invokes the callback. The request will + // remain pending until it is cleaned up. + clipboard_host_impl()->CompleteRequest( + ui::Clipboard::GetForCurrentThread()->GetSequenceNumber( + ui::ClipboardBuffer::kCopyPaste), + base::UTF16ToUTF8(kText)); + EXPECT_EQ( + 1u, + clipboard_host_impl()->is_paste_allowed_requests_for_testing().size()); + + // When a document reads from the clipboard, but the clipboard was written + // from an unknown source, content checks should not be skipped. + EXPECT_EQ(1u, clipboard_host_impl()->start_count()); + EXPECT_EQ(kText, read_text); +} + TEST_F(ClipboardHostImplScanTest, PasteIfPolicyAllowed_EmptyData) { int count = 0;