Commit a8234d9a authored by Ken Rockot's avatar Ken Rockot Committed by Commit Bot

[mojo] Don't crash on BigBuffer allocation failure

A CHECK was added to disambiguate certain large IPC cases from others.
The gist is that BigBuffer fields attempt to use shared memory, but fall
back on inlined data when shared memory allocation fails. This can
result in oversized IPC messages being sent and causing intentional
crashes.

This CL removes the CHECK in favor of sending an invalid BigBuffer
(tagged with a special union field) in cases where shared memory
allocation fails and we think the message is too large to fall back
onto inline encoding. The deserialization traits for BigBuffer and
BigBufferView will always reject such invalid BigBuffer encodings,
resulting in an interface error.

Also updates RenderFrameImpl to tolerate its ClipboardHostPtr seeing
errors as a result of this validation failure mode.

Bug: 900113
Change-Id: Ia66e9cee00819f0d61e198051b53a35c5150b399
Reviewed-on: https://chromium-review.googlesource.com/c/1308095
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604431}
parent 5dc1b344
...@@ -2344,11 +2344,17 @@ void RenderFrameImpl::OnCopyToFindPboard() { ...@@ -2344,11 +2344,17 @@ void RenderFrameImpl::OnCopyToFindPboard() {
auto* platform = RenderThreadImpl::current_blink_platform_impl(); auto* platform = RenderThreadImpl::current_blink_platform_impl();
platform->GetConnector()->BindInterface(platform->GetBrowserServiceName(), platform->GetConnector()->BindInterface(platform->GetBrowserServiceName(),
&clipboard_host_); &clipboard_host_);
clipboard_host_.set_connection_error_handler(base::BindOnce(
&RenderFrameImpl::OnClipboardHostError, base::Unretained(this)));
} }
base::string16 selection = frame_->SelectionAsText().Utf16(); base::string16 selection = frame_->SelectionAsText().Utf16();
clipboard_host_->WriteStringToFindPboard(selection); clipboard_host_->WriteStringToFindPboard(selection);
} }
} }
void RenderFrameImpl::OnClipboardHostError() {
clipboard_host_.reset();
}
#endif #endif
void RenderFrameImpl::OnCopyImageAt(int x, int y) { void RenderFrameImpl::OnCopyImageAt(int x, int y) {
......
...@@ -881,6 +881,7 @@ class CONTENT_EXPORT RenderFrameImpl ...@@ -881,6 +881,7 @@ class CONTENT_EXPORT RenderFrameImpl
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
void OnCopyToFindPboard(); void OnCopyToFindPboard();
void OnClipboardHostError();
#endif #endif
// Dispatches the current state of selection on the webpage to the browser if // Dispatches the current state of selection on the webpage to the browser if
......
...@@ -8,6 +8,15 @@ ...@@ -8,6 +8,15 @@
namespace mojo_base { namespace mojo_base {
namespace {
// In the case of shared memory allocation failure, we still attempt to fall
// back onto inline bytes unless the buffer size exceeds a very large threshold,
// given by this constant.
constexpr size_t kMaxFallbackInlineBytes = 127 * 1024 * 1024;
} // namespace
namespace internal { namespace internal {
BigBufferSharedMemoryRegion::BigBufferSharedMemoryRegion() = default; BigBufferSharedMemoryRegion::BigBufferSharedMemoryRegion() = default;
...@@ -69,6 +78,10 @@ const uint8_t* BigBuffer::data() const { ...@@ -69,6 +78,10 @@ const uint8_t* BigBuffer::data() const {
DCHECK(shared_memory_->buffer_mapping_); DCHECK(shared_memory_->buffer_mapping_);
return static_cast<const uint8_t*>( return static_cast<const uint8_t*>(
const_cast<const void*>(shared_memory_->buffer_mapping_.get())); const_cast<const void*>(shared_memory_->buffer_mapping_.get()));
case StorageType::kInvalidBuffer:
// We return null here but do not assert unlike the default case. No
// consumer is allowed to dereference this when |size()| is zero anyway.
return nullptr;
default: default:
NOTREACHED(); NOTREACHED();
return nullptr; return nullptr;
...@@ -81,6 +94,8 @@ size_t BigBuffer::size() const { ...@@ -81,6 +94,8 @@ size_t BigBuffer::size() const {
return bytes_.size(); return bytes_.size();
case StorageType::kSharedMemory: case StorageType::kSharedMemory:
return shared_memory_->size(); return shared_memory_->size();
case StorageType::kInvalidBuffer:
return 0;
default: default:
NOTREACHED(); NOTREACHED();
return 0; return 0;
...@@ -102,11 +117,13 @@ BigBufferView::BigBufferView(base::span<const uint8_t> bytes) { ...@@ -102,11 +117,13 @@ BigBufferView::BigBufferView(base::span<const uint8_t> bytes) {
return; return;
} }
// Shared memory allocation failed, so we're going to inline the data. If if (bytes.size() > kMaxFallbackInlineBytes) {
// the data is large enough to be rejected by Mojo internals, we crash early // The data is too large to even bother with inline fallback, so we
// to disambiguate this case from other intentional large-IPC crashes. See // instead produce an invalid buffer. This will always fail validation on
// https://crbug.com/872237. // the receiving end.
CHECK_LE(bytes.size(), 127u * 1024 * 1024); storage_type_ = BigBuffer::StorageType::kInvalidBuffer;
return;
}
} }
// Either the data is small enough or shared memory allocation failed. Either // Either the data is small enough or shared memory allocation failed. Either
...@@ -137,12 +154,14 @@ void BigBufferView::SetSharedMemory( ...@@ -137,12 +154,14 @@ void BigBufferView::SetSharedMemory(
base::span<const uint8_t> BigBufferView::data() const { base::span<const uint8_t> BigBufferView::data() const {
if (storage_type_ == BigBuffer::StorageType::kBytes) { if (storage_type_ == BigBuffer::StorageType::kBytes) {
return bytes_; return bytes_;
} else { } else if (storage_type_ == BigBuffer::StorageType::kSharedMemory) {
DCHECK(shared_memory_.has_value()); DCHECK(shared_memory_.has_value());
return base::make_span(static_cast<const uint8_t*>(const_cast<const void*>( return base::make_span(static_cast<const uint8_t*>(const_cast<const void*>(
shared_memory_->memory())), shared_memory_->memory())),
shared_memory_->size()); shared_memory_->size());
} }
return base::span<const uint8_t>();
} }
// static // static
...@@ -152,10 +171,17 @@ BigBuffer BigBufferView::ToBigBuffer(BigBufferView view) { ...@@ -152,10 +171,17 @@ BigBuffer BigBufferView::ToBigBuffer(BigBufferView view) {
if (view.storage_type_ == BigBuffer::StorageType::kBytes) { if (view.storage_type_ == BigBuffer::StorageType::kBytes) {
std::copy(view.bytes_.begin(), view.bytes_.end(), std::copy(view.bytes_.begin(), view.bytes_.end(),
std::back_inserter(buffer.bytes_)); std::back_inserter(buffer.bytes_));
} else { } else if (view.storage_type_ == BigBuffer::StorageType::kSharedMemory) {
buffer.shared_memory_ = std::move(*view.shared_memory_); buffer.shared_memory_ = std::move(*view.shared_memory_);
} }
return buffer; return buffer;
} }
// static
BigBufferView BigBufferView::CreateInvalidForTest() {
BigBufferView view;
view.storage_type_ = BigBuffer::StorageType::kInvalidBuffer;
return view;
}
} // namespace mojo_base } // namespace mojo_base
...@@ -69,6 +69,7 @@ class COMPONENT_EXPORT(MOJO_BASE) BigBuffer { ...@@ -69,6 +69,7 @@ class COMPONENT_EXPORT(MOJO_BASE) BigBuffer {
enum class StorageType { enum class StorageType {
kBytes, kBytes,
kSharedMemory, kSharedMemory,
kInvalidBuffer,
}; };
// Defaults to empty vector storage. // Defaults to empty vector storage.
...@@ -166,6 +167,8 @@ class COMPONENT_EXPORT(MOJO_BASE) BigBufferView { ...@@ -166,6 +167,8 @@ class COMPONENT_EXPORT(MOJO_BASE) BigBufferView {
return shared_memory_.value(); return shared_memory_.value();
} }
static BigBufferView CreateInvalidForTest();
private: private:
BigBuffer::StorageType storage_type_; BigBuffer::StorageType storage_type_;
base::span<const uint8_t> bytes_; base::span<const uint8_t> bytes_;
......
...@@ -46,6 +46,8 @@ UnionTraits<mojo_base::mojom::BigBufferDataView, mojo_base::BigBuffer>::GetTag( ...@@ -46,6 +46,8 @@ UnionTraits<mojo_base::mojom::BigBufferDataView, mojo_base::BigBuffer>::GetTag(
return mojo_base::mojom::BigBufferDataView::Tag::BYTES; return mojo_base::mojom::BigBufferDataView::Tag::BYTES;
case mojo_base::BigBuffer::StorageType::kSharedMemory: case mojo_base::BigBuffer::StorageType::kSharedMemory:
return mojo_base::mojom::BigBufferDataView::Tag::SHARED_MEMORY; return mojo_base::mojom::BigBufferDataView::Tag::SHARED_MEMORY;
case mojo_base::BigBuffer::StorageType::kInvalidBuffer:
return mojo_base::mojom::BigBufferDataView::Tag::INVALID_BUFFER;
} }
NOTREACHED(); NOTREACHED();
...@@ -66,6 +68,13 @@ UnionTraits<mojo_base::mojom::BigBufferDataView, ...@@ -66,6 +68,13 @@ UnionTraits<mojo_base::mojom::BigBufferDataView,
return buffer.shared_memory(); return buffer.shared_memory();
} }
// static
bool UnionTraits<mojo_base::mojom::BigBufferDataView,
mojo_base::BigBuffer>::invalid_buffer(mojo_base::BigBuffer&
buffer) {
return true;
}
// static // static
bool UnionTraits<mojo_base::mojom::BigBufferDataView, mojo_base::BigBuffer>:: bool UnionTraits<mojo_base::mojom::BigBufferDataView, mojo_base::BigBuffer>::
Read(mojo_base::mojom::BigBufferDataView data, mojo_base::BigBuffer* out) { Read(mojo_base::mojom::BigBufferDataView data, mojo_base::BigBuffer* out) {
...@@ -84,6 +93,10 @@ bool UnionTraits<mojo_base::mojom::BigBufferDataView, mojo_base::BigBuffer>:: ...@@ -84,6 +93,10 @@ bool UnionTraits<mojo_base::mojom::BigBufferDataView, mojo_base::BigBuffer>::
*out = mojo_base::BigBuffer(std::move(shared_memory)); *out = mojo_base::BigBuffer(std::move(shared_memory));
return true; return true;
} }
case mojo_base::mojom::BigBufferDataView::Tag::INVALID_BUFFER:
// Always reject an invalid buffer in deserialization.
return false;
} }
return false; return false;
...@@ -98,6 +111,8 @@ mojo_base::mojom::BigBufferDataView::Tag UnionTraits< ...@@ -98,6 +111,8 @@ mojo_base::mojom::BigBufferDataView::Tag UnionTraits<
return mojo_base::mojom::BigBufferDataView::Tag::BYTES; return mojo_base::mojom::BigBufferDataView::Tag::BYTES;
case mojo_base::BigBuffer::StorageType::kSharedMemory: case mojo_base::BigBuffer::StorageType::kSharedMemory:
return mojo_base::mojom::BigBufferDataView::Tag::SHARED_MEMORY; return mojo_base::mojom::BigBufferDataView::Tag::SHARED_MEMORY;
case mojo_base::BigBuffer::StorageType::kInvalidBuffer:
return mojo_base::mojom::BigBufferDataView::Tag::INVALID_BUFFER;
} }
NOTREACHED(); NOTREACHED();
...@@ -118,6 +133,13 @@ mojo_base::internal::BigBufferSharedMemoryRegion& UnionTraits< ...@@ -118,6 +133,13 @@ mojo_base::internal::BigBufferSharedMemoryRegion& UnionTraits<
return view.shared_memory(); return view.shared_memory();
} }
// static
bool UnionTraits<mojo_base::mojom::BigBufferDataView,
mojo_base::BigBufferView>::
invalid_buffer(mojo_base::BigBufferView& buffer) {
return true;
}
// static // static
bool UnionTraits< bool UnionTraits<
mojo_base::mojom::BigBufferDataView, mojo_base::mojom::BigBufferDataView,
...@@ -138,6 +160,10 @@ bool UnionTraits< ...@@ -138,6 +160,10 @@ bool UnionTraits<
out->SetSharedMemory(std::move(shared_memory)); out->SetSharedMemory(std::move(shared_memory));
return true; return true;
} }
case mojo_base::mojom::BigBufferDataView::Tag::INVALID_BUFFER:
// Always reject an invalid buffer in deserialization.
return false;
} }
return false; return false;
......
...@@ -40,6 +40,7 @@ struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS) ...@@ -40,6 +40,7 @@ struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS)
static const std::vector<uint8_t>& bytes(const mojo_base::BigBuffer& buffer); static const std::vector<uint8_t>& bytes(const mojo_base::BigBuffer& buffer);
static mojo_base::internal::BigBufferSharedMemoryRegion& shared_memory( static mojo_base::internal::BigBufferSharedMemoryRegion& shared_memory(
mojo_base::BigBuffer& buffer); mojo_base::BigBuffer& buffer);
static bool invalid_buffer(mojo_base::BigBuffer& buffer);
static bool Read(mojo_base::mojom::BigBufferDataView data, static bool Read(mojo_base::mojom::BigBufferDataView data,
mojo_base::BigBuffer* out); mojo_base::BigBuffer* out);
...@@ -54,6 +55,7 @@ struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS) ...@@ -54,6 +55,7 @@ struct COMPONENT_EXPORT(MOJO_BASE_SHARED_TRAITS)
static base::span<const uint8_t> bytes(const mojo_base::BigBufferView& view); static base::span<const uint8_t> bytes(const mojo_base::BigBufferView& view);
static mojo_base::internal::BigBufferSharedMemoryRegion& shared_memory( static mojo_base::internal::BigBufferSharedMemoryRegion& shared_memory(
mojo_base::BigBufferView& view); mojo_base::BigBufferView& view);
static bool invalid_buffer(mojo_base::BigBufferView& buffer);
static bool Read(mojo_base::mojom::BigBufferDataView data, static bool Read(mojo_base::mojom::BigBufferDataView data,
mojo_base::BigBufferView* out); mojo_base::BigBufferView* out);
......
...@@ -65,5 +65,25 @@ TEST(BigBufferTest, LargeDataSize) { ...@@ -65,5 +65,25 @@ TEST(BigBufferTest, LargeDataSize) {
EXPECT_TRUE(BufferEquals(data, out)); EXPECT_TRUE(BufferEquals(data, out));
} }
TEST(BigBufferTest, InvalidBuffer) {
// Verifies that deserializing invalid BigBuffers and BigBufferViews always
// fails.
BigBufferView out_view;
auto invalid_view = BigBufferView::CreateInvalidForTest();
EXPECT_EQ(invalid_view.storage_type(),
BigBuffer::StorageType::kInvalidBuffer);
EXPECT_FALSE(mojo::test::SerializeAndDeserialize<mojom::BigBuffer>(
&invalid_view, &out_view));
BigBuffer out_buffer;
auto invalid_buffer =
BigBufferView::ToBigBuffer(BigBufferView::CreateInvalidForTest());
EXPECT_EQ(invalid_buffer.storage_type(),
BigBuffer::StorageType::kInvalidBuffer);
EXPECT_FALSE(mojo::test::SerializeAndDeserialize<mojom::BigBuffer>(
&invalid_buffer, &out_buffer));
}
} // namespace big_buffer_unittest } // namespace big_buffer_unittest
} // namespace mojo_base } // namespace mojo_base
...@@ -11,8 +11,12 @@ struct BigBufferSharedMemoryRegion { ...@@ -11,8 +11,12 @@ struct BigBufferSharedMemoryRegion {
// A helper union to be used when messages want to accept arbitrarily large // A helper union to be used when messages want to accept arbitrarily large
// chunks of byte data. Beyond a certain size threshold, shared memory will be // chunks of byte data. Beyond a certain size threshold, shared memory will be
// used in lieu of an inline byte array. // used in lieu of an inline byte array. If shared memory could not be allocated
// by the sender and the contents of the buffer would be too large to inline,
// this will instead take on the value of |invalid_buffer=true| indicating that
// the buffer does not represent any contents intended by the sender.
union BigBuffer { union BigBuffer {
array<uint8> bytes; array<uint8> bytes;
BigBufferSharedMemoryRegion shared_memory; BigBufferSharedMemoryRegion shared_memory;
bool invalid_buffer;
}; };
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