Commit 3c91f864 authored by Derek Cheng's avatar Derek Cheng Committed by Commit Bot

[Cast channel Fix MessageFramer body size check.

The cast protocol allows a max message size of 64kb = 65536 bytes.
However Chrome does not enforce this correctly in the following ways:
- 65535 is used instead of 65536 as the max limit
- The limit should not include the 4 byte header, but Chrome includes it
(e.g., the IOBuffer used to read both only has 65535 bytes of capacity)

This patch fixes this by allocating 65536 + 4 bytes for the input
buffer, and modifying the check for message size against 65536.

Bug: 822611
Change-Id: I27121abb57dee5aee9741173e19cf0178184477d
Reviewed-on: https://chromium-review.googlesource.com/996475
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: default avatarAdam Parker <amp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548860}
parent a03566bc
...@@ -57,9 +57,14 @@ size_t MessageFramer::MessageHeader::header_size() { ...@@ -57,9 +57,14 @@ size_t MessageFramer::MessageHeader::header_size() {
return sizeof(uint32_t); return sizeof(uint32_t);
} }
// static
size_t MessageFramer::MessageHeader::max_body_size() {
return 65536;
}
// static // static
size_t MessageFramer::MessageHeader::max_message_size() { size_t MessageFramer::MessageHeader::max_message_size() {
return 65535; return header_size() + max_body_size();
} }
std::string MessageFramer::MessageHeader::ToString() { std::string MessageFramer::MessageHeader::ToString() {
...@@ -73,7 +78,7 @@ bool MessageFramer::Serialize(const CastMessage& message_proto, ...@@ -73,7 +78,7 @@ bool MessageFramer::Serialize(const CastMessage& message_proto,
DCHECK(message_data); DCHECK(message_data);
message_proto.SerializeToString(message_data); message_proto.SerializeToString(message_data);
size_t message_size = message_data->size(); size_t message_size = message_data->size();
if (message_size > MessageHeader::max_message_size()) { if (message_size > MessageHeader::max_body_size()) {
message_data->clear(); message_data->clear();
return false; return false;
} }
...@@ -98,8 +103,7 @@ size_t MessageFramer::BytesRequested() { ...@@ -98,8 +103,7 @@ size_t MessageFramer::BytesRequested() {
case BODY: case BODY:
bytes_left = bytes_left =
(body_size_ + MessageHeader::header_size()) - message_bytes_received_; (body_size_ + MessageHeader::header_size()) - message_bytes_received_;
DCHECK_LE(bytes_left, MessageHeader::max_message_size() - DCHECK_LE(bytes_left, MessageHeader::max_body_size());
MessageHeader::header_size());
VLOG(2) << "Bytes needed for body: " << bytes_left; VLOG(2) << "Bytes needed for body: " << bytes_left;
return bytes_left; return bytes_left;
default: default:
...@@ -129,7 +133,7 @@ std::unique_ptr<CastMessage> MessageFramer::Ingest(size_t num_bytes, ...@@ -129,7 +133,7 @@ std::unique_ptr<CastMessage> MessageFramer::Ingest(size_t num_bytes,
if (BytesRequested() == 0) { if (BytesRequested() == 0) {
MessageHeader header; MessageHeader header;
MessageHeader::Deserialize(input_buffer_->StartOfBuffer(), &header); MessageHeader::Deserialize(input_buffer_->StartOfBuffer(), &header);
if (header.message_size > MessageHeader::max_message_size()) { if (header.message_size > MessageHeader::max_body_size()) {
VLOG(1) << "Error parsing header (message size too large)."; VLOG(1) << "Error parsing header (message size too large).";
*error = ChannelError::INVALID_MESSAGE; *error = ChannelError::INVALID_MESSAGE;
error_ = true; error_ = true;
......
...@@ -67,6 +67,8 @@ class MessageFramer { ...@@ -67,6 +67,8 @@ class MessageFramer {
static size_t header_size(); static size_t header_size();
// Maximum size (in bytes) of a message payload on the wire (does not // Maximum size (in bytes) of a message payload on the wire (does not
// include header). // include header).
static size_t max_body_size();
// Maximum size (in bytes) of a message (header + payload) on the wire.
static size_t max_message_size(); static size_t max_message_size();
std::string ToString(); std::string ToString();
// The size of the following protocol message in bytes, in host byte order. // The size of the following protocol message in bytes, in host byte order.
......
...@@ -81,7 +81,7 @@ TEST_F(CastFramerTest, TestSerializeErrorMessageTooLarge) { ...@@ -81,7 +81,7 @@ TEST_F(CastFramerTest, TestSerializeErrorMessageTooLarge) {
CastMessage big_message; CastMessage big_message;
big_message.CopyFrom(cast_message_); big_message.CopyFrom(cast_message_);
std::string payload; std::string payload;
payload.append(MessageFramer::MessageHeader::max_message_size() + 1, 'x'); payload.append(MessageFramer::MessageHeader::max_body_size() + 1, 'x');
big_message.set_payload_utf8(payload); big_message.set_payload_utf8(payload);
EXPECT_FALSE(MessageFramer::Serialize(big_message, &serialized)); EXPECT_FALSE(MessageFramer::Serialize(big_message, &serialized));
} }
...@@ -109,6 +109,30 @@ TEST_F(CastFramerTest, TestIngestIllegalLargeMessage) { ...@@ -109,6 +109,30 @@ TEST_F(CastFramerTest, TestIngestIllegalLargeMessage) {
EXPECT_EQ(0u, framer_->BytesRequested()); EXPECT_EQ(0u, framer_->BytesRequested());
} }
TEST_F(CastFramerTest, TestIngestIllegalLargeMessage2) {
std::string mangled_cast_message = cast_message_str_;
// Header indicates body size is 0x00010001 = 65537
mangled_cast_message[0] = 0;
mangled_cast_message[1] = 0x1;
mangled_cast_message[2] = 0;
mangled_cast_message[3] = 0x1;
WriteToBuffer(mangled_cast_message);
size_t bytes_ingested;
ChannelError error;
EXPECT_EQ(4u, framer_->BytesRequested());
EXPECT_EQ(nullptr, framer_->Ingest(4, &bytes_ingested, &error).get());
EXPECT_EQ(ChannelError::INVALID_MESSAGE, error);
EXPECT_EQ(0u, framer_->BytesRequested());
// Test that the parser enters a terminal error state.
WriteToBuffer(cast_message_str_);
EXPECT_EQ(0u, framer_->BytesRequested());
EXPECT_EQ(nullptr, framer_->Ingest(4, &bytes_ingested, &error).get());
EXPECT_EQ(ChannelError::INVALID_MESSAGE, error);
EXPECT_EQ(0u, framer_->BytesRequested());
}
TEST_F(CastFramerTest, TestUnparsableBodyProto) { TEST_F(CastFramerTest, TestUnparsableBodyProto) {
// Message header is OK, but the body is replaced with "x"en. // Message header is OK, but the body is replaced with "x"en.
std::string mangled_cast_message = cast_message_str_; std::string mangled_cast_message = cast_message_str_;
......
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