diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-04-29 09:48:59 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-04-29 07:57:35 +0000 |
commit | 6f260e5b2f717b9977045419f12c7c35d31f9a90 (patch) | |
tree | 4f06cb50b7721ee7ad8bf1ad8dabfd32e149e797 | |
parent | 165753b378146fbfb354d30f8ebb83d2ad1a1313 (diff) |
[Backport] CVE-2020-6461: Use after free in storagev5.15.0-rc1
[Blobs] Fix bug when BytesProvider replies with invalid data.
(cherry picked from commit 38990b7d56e6dde6bfdc2d81950db8ddef4e4116)
Bug: 1072983
Change-Id: Ideaa0a67680375e770995880a4b8d2014b51d642
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#761203}
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/branch-heads/4044@{#972}
Cr-Branched-From: a6d9daf149a473ceea37f629c41d4527bf2055bd-refs/heads/master@{#737173}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r-- | chromium/storage/browser/blob/blob_registry_impl.cc | 6 | ||||
-rw-r--r-- | chromium/storage/browser/blob/blob_registry_impl_unittest.cc | 30 |
2 files changed, 35 insertions, 1 deletions
diff --git a/chromium/storage/browser/blob/blob_registry_impl.cc b/chromium/storage/browser/blob/blob_registry_impl.cc index 1cc34f91e01..b6a12a5c43d 100644 --- a/chromium/storage/browser/blob/blob_registry_impl.cc +++ b/chromium/storage/browser/blob/blob_registry_impl.cc @@ -432,6 +432,10 @@ void BlobRegistryImpl::BlobUnderConstruction::TransportComplete( // try to delete |this| again afterwards. auto weak_this = weak_ptr_factory_.GetWeakPtr(); + // Store the bad_message_callback_, so we can invoke it if needed, after + // notifying about the blob being finished. + auto bad_message_callback = std::move(bad_message_callback_); + // The blob might no longer have any references, in which case it may no // longer exist. If that happens just skip calling Complete. // TODO(mek): Stop building sooner if a blob is no longer referenced. @@ -445,7 +449,7 @@ void BlobRegistryImpl::BlobUnderConstruction::TransportComplete( // BlobTransportStrategy might have already reported a BadMessage on the // BytesProvider binding, but just to be safe, also report one on the // BlobRegistry binding itself. - std::move(bad_message_callback_) + std::move(bad_message_callback) .Run("Received invalid data while transporting blob"); } if (weak_this) diff --git a/chromium/storage/browser/blob/blob_registry_impl_unittest.cc b/chromium/storage/browser/blob/blob_registry_impl_unittest.cc index 8cbcc69ec6d..2060676987e 100644 --- a/chromium/storage/browser/blob/blob_registry_impl_unittest.cc +++ b/chromium/storage/browser/blob/blob_registry_impl_unittest.cc @@ -770,6 +770,36 @@ TEST_F(BlobRegistryImplTest, Register_ValidBytesAsReply) { EXPECT_EQ(0u, BlobsUnderConstruction()); } +TEST_F(BlobRegistryImplTest, Register_InvalidBytesAsReply) { + const std::string kId = "id"; + const std::string kData = "hello"; + + std::vector<blink::mojom::DataElementPtr> elements; + elements.push_back( + blink::mojom::DataElement::NewBytes(blink::mojom::DataElementBytes::New( + kData.size(), base::nullopt, CreateBytesProvider("")))); + + mojo::PendingRemote<blink::mojom::Blob> blob; + EXPECT_TRUE(registry_->Register(blob.InitWithNewPipeAndPassReceiver(), kId, + "", "", std::move(elements))); + + std::unique_ptr<BlobDataHandle> handle = context_->GetBlobDataFromUUID(kId); + WaitForBlobCompletion(handle.get()); + + EXPECT_TRUE(handle->IsBroken()); + ASSERT_EQ(BlobStatus::ERR_INVALID_CONSTRUCTION_ARGUMENTS, + handle->GetBlobStatus()); + + EXPECT_EQ(1u, reply_request_count_); + EXPECT_EQ(0u, stream_request_count_); + EXPECT_EQ(0u, file_request_count_); + EXPECT_EQ(0u, BlobsUnderConstruction()); + + // Expect 2 bad messages, one for the bad reply by the bytes provider, and one + // for the original register call. + EXPECT_EQ(2u, bad_messages_.size()); +} + TEST_F(BlobRegistryImplTest, Register_ValidBytesAsStream) { const std::string kId = "id"; const std::string kData = |