Mojo EDK: Fix MojoExtendSerializedMessagePayload

There are two problems with this API:

1. The buffer size returned is incorrect and could trivially
lead to callers overflowing the message buffer.

2. It does not correctly copy the entire contents of the old message
buffer upon reallocation, instead only copying the extent of the buffer
reported as payload so far.

This fixes both problems. Note that this API has not yet been used in
production, so there are no actual bugs caused by either of these
issues yet.

BUG=742369
R=jcivelli@chromium.org

Change-Id: I7ee7cd0783d641940e0ccafc0c9dc772eeaf2793
Reviewed-on: https://chromium-review.googlesource.com/580252Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488769}

mojo/edk/system/channel.cc

@@ -265,7 +265,7 @@ void Channel::Message::ExtendPayload(size_t new_payload_size) {
     size_t new_capacity =
         std::max(capacity_without_header * 2, new_payload_size) + header_size;
     void* new_data = base::AlignedAlloc(new_capacity, kChannelMessageAlignment);
-    memcpy(new_data, data_, size_);
+    memcpy(new_data, data_, capacity_);
     base::AlignedFree(data_);
     data_ = static_cast<char*>(new_data);
     capacity_ = new_capacity;

mojo/edk/system/core.cc

@@ -505,8 +505,7 @@ MojoResult Core::AttachSerializedMessageBuffer(MojoMessageHandle message_handle,
     return rv;
 
   *buffer = message->user_payload();
-  *buffer_size =
-      base::checked_cast<uint32_t>(message->user_payload_buffer_size());
+  *buffer_size = base::checked_cast<uint32_t>(message->user_payload_capacity());
   return MOJO_RESULT_OK;
 }
 
@@ -525,7 +524,7 @@ MojoResult Core::ExtendSerializedMessagePayload(
 
   *new_buffer = message->user_payload();
   *new_buffer_size =
-      base::checked_cast<uint32_t>(message->user_payload_buffer_size());
+      base::checked_cast<uint32_t>(message->user_payload_capacity());
   return MOJO_RESULT_OK;
 }
 

mojo/edk/system/message_unittest.cc

@@ -704,6 +704,36 @@ TEST_F(MessageTest, ExtendMessagePayloadLarge) {
   }
 }
 
+TEST_F(MessageTest, CorrectPayloadBufferBoundaries) {
+  // Exercises writes to the full extent of a message's payload under various
+  // circumstances in an effort to catch any potential bugs in internal
+  // allocations or reported size from Mojo APIs.
+
+  MojoMessageHandle message;
+  void* buffer = nullptr;
+  uint32_t buffer_size = 0;
+  EXPECT_EQ(MOJO_RESULT_OK, MojoCreateMessage(&message));
+  EXPECT_EQ(MOJO_RESULT_OK, MojoAttachSerializedMessageBuffer(
+                                message, 0, nullptr, 0, &buffer, &buffer_size));
+  // Fill the buffer end-to-end.
+  memset(buffer, 'x', buffer_size);
+
+  // Continuously grow and fill the message buffer several more times. Should
+  // not crash.
+  uint32_t payload_size = 0;
+  constexpr uint32_t kChunkSize = 4096;
+  constexpr size_t kNumIterations = 1000;
+  for (size_t i = 0; i < kNumIterations; ++i) {
+    payload_size += kChunkSize;
+    EXPECT_EQ(MOJO_RESULT_OK,
+              MojoExtendSerializedMessagePayload(message, payload_size, &buffer,
+                                                 &buffer_size));
+    memset(buffer, 'x', buffer_size);
+  }
+
+  EXPECT_EQ(MOJO_RESULT_OK, MojoDestroyMessage(message));
+}
+
 }  // namespace
 }  // namespace edk
 }  // namespace mojo

mojo/edk/system/user_message_impl.cc

@@ -288,9 +288,14 @@ uintptr_t UserMessageImpl::ReleaseContext() {
   return context;
 }
 
-size_t UserMessageImpl::user_payload_buffer_size() const {
+size_t UserMessageImpl::user_payload_capacity() const {
   DCHECK(IsSerialized());
-  return channel_message_->capacity();
+  const size_t user_payload_offset =
+      static_cast<uint8_t*>(user_payload_) -
+      static_cast<const uint8_t*>(channel_message_->payload());
+  const size_t message_capacity = channel_message_->capacity();
+  DCHECK(user_payload_offset < message_capacity);
+  return message_capacity - user_payload_offset;
 }
 
 size_t UserMessageImpl::num_handles() const {

mojo/edk/system/user_message_impl.h

@@ -111,7 +111,7 @@ class MOJO_SYSTEM_IMPL_EXPORT UserMessageImpl
     return user_payload_size_;
   }
 
-  size_t user_payload_buffer_size() const;
+  size_t user_payload_capacity() const;
 
   size_t num_handles() const;