Commit d6567127 authored by Kenton Varda's avatar Kenton Varda

Refuse to read messages with excessive segment counts or messages whose total…

Refuse to read messages with excessive segment counts or messages whose total size exceeds the traversal limit, before allocating any space.  This is important for security.
parent 753882f9
...@@ -287,6 +287,41 @@ TEST(Serialize, FileDescriptors) { ...@@ -287,6 +287,41 @@ TEST(Serialize, FileDescriptors) {
} }
} }
TEST(Serialize, RejectTooManySegments) {
Array<word> data = newArray<word>(8192);
WireValue<uint32_t>* table = reinterpret_cast<WireValue<uint32_t>*>(data.begin());
table[0].set(1024);
for (uint i = 0; i < 1024; i++) {
table[i+1].set(1);
}
TestInputStream input(data.asPtr(), false);
try {
InputStreamMessageReader reader(input);
ADD_FAILURE() << "Should have thrown an exception.";
} catch (...) {
// expected
}
}
TEST(Serialize, RejectHugeMessage) {
// A message whose root struct contains two words of data!
AlignedData<4> data = {{0,0,0,0,3,0,0,0, 0,0,0,0,2,0,0,0, 0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0}};
TestInputStream input(arrayPtr(data.words, 4), false);
// We'll set the traversal limit to 2 words so our 3-word message is too big.
ReaderOptions options;
options.traversalLimitInWords = 2;
try {
InputStreamMessageReader reader(input, options);
ADD_FAILURE() << "Should have thrown an exception.";
} catch (...) {
// expected
}
}
// TODO: Test error cases. // TODO: Test error cases.
} // namespace } // namespace
......
...@@ -140,6 +140,9 @@ InputStreamMessageReader::InputStreamMessageReader( ...@@ -140,6 +140,9 @@ InputStreamMessageReader::InputStreamMessageReader(
size_t totalWords = segment0Size; size_t totalWords = segment0Size;
// Reject messages with too many segments for security reasons.
CAPNPROTO_ASSERT(segmentCount < 512, "Message has too many segments.");
// Read sizes for all segments except the first. Include padding if necessary. // Read sizes for all segments except the first. Include padding if necessary.
internal::WireValue<uint32_t> moreSizes[segmentCount & ~1]; internal::WireValue<uint32_t> moreSizes[segmentCount & ~1];
if (segmentCount > 1) { if (segmentCount > 1) {
...@@ -149,6 +152,13 @@ InputStreamMessageReader::InputStreamMessageReader( ...@@ -149,6 +152,13 @@ InputStreamMessageReader::InputStreamMessageReader(
} }
} }
// Don't accept a message which the receiver couldn't possibly traverse without hitting the
// traversal limit. Without this check, a malicious client could transmit a very large segment
// size to make the receiver allocate excessive space and possibly crash.
CAPNPROTO_ASSERT(totalWords <= options.traversalLimitInWords,
"Message is too large. To increase the limit on the receiving end, see "
"capnproto::ReaderOptions.");
if (scratchSpace.size() < totalWords) { if (scratchSpace.size() < totalWords) {
// TODO: Consider allocating each segment as a separate chunk to reduce memory fragmentation. // TODO: Consider allocating each segment as a separate chunk to reduce memory fragmentation.
ownedSpace = newArray<word>(totalWords); ownedSpace = newArray<word>(totalWords);
......
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