From 29e0f2f9ad9a9f377995804efaf72476cc987714 Mon Sep 17 00:00:00 2001 From: erikchen Date: Fri, 11 Sep 2015 12:08:53 -0700 Subject: [PATCH] ipc: Update Message::FindNext to parse brokered attachments. The method Message::FindNext now takes an output parameter NextMessageInfo which contains additional metadata needed for generating brokered attachments. BUG=493414 Review URL: https://codereview.chromium.org/1308613006 Cr-Commit-Position: refs/heads/master@{#348459} --- ipc/ipc_channel_reader.cc | 16 ++++--- ipc/ipc_message.cc | 53 ++++++++++++++++++++++ ipc/ipc_message.h | 33 ++++++++++++-- .../ipc_fuzzer/message_lib/message_file_reader.cc | 8 ++-- 4 files changed, 96 insertions(+), 14 deletions(-) diff --git a/ipc/ipc_channel_reader.cc b/ipc/ipc_channel_reader.cc index f80d75d8e1ee..b3dd0b704d9f 100644 --- a/ipc/ipc_channel_reader.cc +++ b/ipc/ipc_channel_reader.cc @@ -91,11 +91,15 @@ bool ChannelReader::TranslateInputData(const char* input_data, // Dispatch all complete messages in the data buffer. while (p < end) { - const char* message_tail = Message::FindNext(p, end); - if (message_tail) { - int len = static_cast(message_tail - p); + Message::NextMessageInfo info; + Message::FindNext(p, end, &info); + if (info.message_found) { + int pickle_len = static_cast(info.pickle_end - p); + Message translated_message(p, pickle_len); + + // TODO(erikchen): Make attachments for info.attachment_ids. + // http://crbug.com/493414. - Message translated_message(p, len); if (!GetNonBrokeredAttachments(&translated_message)) return false; @@ -109,7 +113,7 @@ bool ChannelReader::TranslateInputData(const char* input_data, if (blocked_ids.empty()) { // Dispatch the message and continue the loop. DispatchMessage(&translated_message); - p = message_tail; + p = info.message_end; continue; } @@ -120,7 +124,7 @@ bool ChannelReader::TranslateInputData(const char* input_data, // Make a deep copy of |translated_message| to add to the queue. scoped_ptr m(new Message(translated_message)); queued_messages_.push_back(m.release()); - p = message_tail; + p = info.message_end; } else { // Last message is partial. break; diff --git a/ipc/ipc_message.cc b/ipc/ipc_message.cc index 2229cdd6a06a..cba89545f875 100644 --- a/ipc/ipc_message.cc +++ b/ipc/ipc_message.cc @@ -4,6 +4,8 @@ #include "ipc/ipc_message.h" +#include + #include "base/atomic_sequence_num.h" #include "base/logging.h" #include "build/build_config.h" @@ -127,6 +129,57 @@ void Message::set_received_time(int64_t time) const { } #endif +Message::NextMessageInfo::NextMessageInfo() + : message_found(false), pickle_end(nullptr), message_end(nullptr) {} +Message::NextMessageInfo::~NextMessageInfo() {} + +// static +void Message::FindNext(const char* range_start, + const char* range_end, + NextMessageInfo* info) { + DCHECK(info); + const char* pickle_end = + base::Pickle::FindNext(sizeof(Header), range_start, range_end); + if (!pickle_end) + return; + info->pickle_end = pickle_end; + +#if USE_ATTACHMENT_BROKER + // The data is not copied. + size_t pickle_len = static_cast(pickle_end - range_start); + Message message(range_start, static_cast(pickle_len)); + int num_attachments = message.header()->num_brokered_attachments; + + // Check for possible overflows. + size_t max_size_t = std::numeric_limits::max(); + if (num_attachments >= max_size_t / BrokerableAttachment::kNonceSize) + return; + + size_t attachment_length = num_attachments * BrokerableAttachment::kNonceSize; + if (pickle_len > max_size_t - attachment_length) + return; + + // Check whether the range includes the attachments. + size_t buffer_length = static_cast(range_end - range_start); + if (buffer_length < attachment_length + pickle_len) + return; + + for (int i = 0; i < num_attachments; ++i) { + const char* attachment_start = + pickle_end + i * BrokerableAttachment::kNonceSize; + BrokerableAttachment::AttachmentId id(attachment_start, + BrokerableAttachment::kNonceSize); + info->attachment_ids.push_back(id); + } + info->message_end = + pickle_end + num_attachments * BrokerableAttachment::kNonceSize; +#else + info->message_end = pickle_end; +#endif // USE_ATTACHMENT_BROKER + + info->message_found = true; +} + bool Message::WriteAttachment(scoped_refptr attachment) { // We write the index of the descriptor so that we don't have to // keep the current descriptor as extra decoding state when deserialising. diff --git a/ipc/ipc_message.h b/ipc/ipc_message.h index cc7ebb31a988..71ead5ea2e45 100644 --- a/ipc/ipc_message.h +++ b/ipc/ipc_message.h @@ -13,6 +13,7 @@ #include "base/memory/ref_counted.h" #include "base/pickle.h" #include "base/trace_event/trace_event.h" +#include "ipc/brokerable_attachment.h" #include "ipc/ipc_export.h" #if !defined(NDEBUG) @@ -166,11 +167,33 @@ class IPC_EXPORT Message : public base::Pickle { static void Log(std::string* name, const Message* msg, std::string* l) { } - // Find the end of the message data that starts at range_start. Returns NULL - // if the entire message is not found in the given data range. - static const char* FindNext(const char* range_start, const char* range_end) { - return base::Pickle::FindNext(sizeof(Header), range_start, range_end); - } + // The static method FindNext() returns several pieces of information, which + // are aggregated into an instance of this struct. + struct NextMessageInfo { + NextMessageInfo(); + ~NextMessageInfo(); + + // Whether an entire message was found in the given memory range. + bool message_found; + // Only filled in if |message_found| is true. + // The start address is passed into FindNext() by the caller, so isn't + // repeated in this struct. The end address of the pickle should be used to + // construct a base::Pickle. + const char* pickle_end; + // Only filled in if |message_found| is true. + // The end address of the message should be used to determine the start + // address of the next message. + const char* message_end; + // If the message has brokerable attachments, this vector will contain the + // ids of the brokerable attachments. The caller of FindNext() is + // responsible for adding the attachments to the message. + std::vector attachment_ids; + }; + + // |info| is an output parameter and must not be nullptr. + static void FindNext(const char* range_start, + const char* range_end, + NextMessageInfo* info); // WriteAttachment appends |attachment| to the end of the set. It returns // false iff the set is full. diff --git a/tools/ipc_fuzzer/message_lib/message_file_reader.cc b/tools/ipc_fuzzer/message_lib/message_file_reader.cc index e93c460b7e28..b8c2bf431497 100644 --- a/tools/ipc_fuzzer/message_lib/message_file_reader.cc +++ b/tools/ipc_fuzzer/message_lib/message_file_reader.cc @@ -108,13 +108,15 @@ bool Reader::ReadMessages() { for (size_t i = 0; i < header_->message_count; ++i) { const char* begin = file_data_.begin(); const char* end = file_data_.end(); - const char* message_tail = IPC::Message::FindNext(begin, end); - if (!message_tail) { + Message::NextMessageInfo info; + IPC::Message::FindNext(begin, end, &info); + if (!info.message_found) { LOG(ERROR) << "Failed to parse message."; return false; } - size_t msglen = message_tail - begin; + CHECK_EQ(info.message_end, info.pickle_end); + size_t msglen = info.message_end - begin; if (msglen > INT_MAX) { LOG(ERROR) << "Message too large."; return false; -- 2.11.4.GIT