Skip to content

Commit 1048706

Browse files
committed
1 parent 26bcced commit 1048706

4 files changed

Lines changed: 87 additions & 3 deletions

File tree

CONTRIBUTORS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Scott Purdy <scott@fer.io>: kj/std iostream interface
1010
Bryan Borham <bjboreham@gmail.com>: Initial MSVC support
1111
Philip Quinn <p@partylemon.com>: cmake build and other assorted bits
1212
Brian Taylor <el.wubo@gmail.com>: emacs syntax highlighting
13+
Ben Laurie <ben@links.org>: discovered and responsibly disclosed security bugs
1314

1415
This file does not list people who maintain their own Cap'n Proto
1516
implementations as separate projects. Those people are awesome too! :)

c++/src/capnp/arena.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,13 @@ class SegmentReader {
117117

118118
KJ_ALWAYS_INLINE(bool containsInterval(const void* from, const void* to));
119119

120+
KJ_ALWAYS_INLINE(bool amplifiedRead(WordCount virtualAmount));
121+
// Indicates that the reader should pretend that `virtualAmount` additional data was read even
122+
// though no actual pointer was traversed. This is used e.g. when reading a struct list pointer
123+
// where the element sizes are zero -- the sender could set the list size arbitrarily high and
124+
// cause the receiver to iterate over this list even though the message itself is small, so we
125+
// need to defend agaisnt DoS attacks based on this.
126+
120127
inline Arena* getArena();
121128
inline SegmentId getSegmentId();
122129

@@ -367,6 +374,10 @@ inline bool SegmentReader::containsInterval(const void* from, const void* to) {
367374
arena);
368375
}
369376

377+
inline bool SegmentReader::amplifiedRead(WordCount virtualAmount) {
378+
return readLimiter->canRead(virtualAmount, arena);
379+
}
380+
370381
inline Arena* SegmentReader::getArena() { return arena; }
371382
inline SegmentId SegmentReader::getSegmentId() { return id; }
372383
inline const word* SegmentReader::getStartPtr() { return ptr.begin(); }

c++/src/capnp/encoding-test.c++

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,6 +1410,36 @@ TEST(Encoding, Has) {
14101410
EXPECT_TRUE(root.asReader().hasInt32List());
14111411
}
14121412

1413+
TEST(Encoding, VoidListAmplification) {
1414+
MallocMessageBuilder builder;
1415+
builder.initRoot<test::TestAnyPointer>().getAnyPointerField().initAs<List<Void>>(1u << 28);
1416+
1417+
auto segments = builder.getSegmentsForOutput();
1418+
EXPECT_EQ(1, segments.size());
1419+
EXPECT_LT(segments[0].size(), 16); // quite small for such a big list!
1420+
1421+
SegmentArrayMessageReader reader(builder.getSegmentsForOutput());
1422+
auto root = reader.getRoot<test::TestAnyPointer>().getAnyPointerField();
1423+
EXPECT_NONFATAL_FAILURE(root.getAs<List<TestAllTypes>>());
1424+
1425+
MallocMessageBuilder copy;
1426+
EXPECT_NONFATAL_FAILURE(copy.setRoot(reader.getRoot<AnyPointer>()));
1427+
}
1428+
1429+
TEST(Encoding, EmptyStructListAmplification) {
1430+
MallocMessageBuilder builder;
1431+
builder.initRoot<test::TestAnyPointer>().getAnyPointerField()
1432+
.initAs<List<test::TestEmptyStruct>>(1u << 28);
1433+
1434+
auto segments = builder.getSegmentsForOutput();
1435+
EXPECT_EQ(1, segments.size());
1436+
EXPECT_LT(segments[0].size(), 16); // quite small for such a big list!
1437+
1438+
SegmentArrayMessageReader reader(builder.getSegmentsForOutput());
1439+
auto root = reader.getRoot<test::TestAnyPointer>().getAnyPointerField();
1440+
EXPECT_NONFATAL_FAILURE(root.getAs<List<TestAllTypes>>());
1441+
}
1442+
14131443
TEST(Encoding, Constants) {
14141444
EXPECT_EQ(VOID, test::TestConstants::VOID_CONST);
14151445
EXPECT_EQ(true, test::TestConstants::BOOL_CONST);

c++/src/capnp/layout.c++

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,11 @@ struct WireHelpers {
308308
return segment == nullptr || segment->containsInterval(start, end);
309309
}
310310

311+
static KJ_ALWAYS_INLINE(bool amplifiedRead(SegmentReader* segment, WordCount virtualAmount)) {
312+
// If segment is null, this is an unchecked message, so we don't do read limiter checks.
313+
return segment == nullptr || segment->amplifiedRead(virtualAmount);
314+
}
315+
311316
static KJ_ALWAYS_INLINE(word* allocate(
312317
WirePointer*& ref, SegmentBuilder*& segment, WordCount amount,
313318
WirePointer::Kind kind, BuilderArena* orphanArena)) {
@@ -1675,6 +1680,15 @@ struct WireHelpers {
16751680
goto useDefault;
16761681
}
16771682

1683+
if (wordsPerElement * (1 * ELEMENTS) == 0 * WORDS) {
1684+
// Watch out for lists of zero-sized structs, which can claim to be arbitrarily large
1685+
// without having sent actual data.
1686+
KJ_REQUIRE(amplifiedRead(srcSegment, elementCount * (1 * WORDS / ELEMENTS)),
1687+
"Message contains amplified list pointer.") {
1688+
goto useDefault;
1689+
}
1690+
}
1691+
16781692
return setListPointer(dstSegment, dst,
16791693
ListReader(srcSegment, ptr, elementCount, wordsPerElement * BITS_PER_WORD,
16801694
tag->structRef.dataSize.get() * BITS_PER_WORD,
@@ -1693,6 +1707,15 @@ struct WireHelpers {
16931707
goto useDefault;
16941708
}
16951709

1710+
if (elementSize == ElementSize::VOID) {
1711+
// Watch out for lists of void, which can claim to be arbitrarily large without having
1712+
// sent actual data.
1713+
KJ_REQUIRE(amplifiedRead(srcSegment, elementCount * (1 * WORDS / ELEMENTS)),
1714+
"Message contains amplified list pointer.") {
1715+
goto useDefault;
1716+
}
1717+
}
1718+
16961719
return setListPointer(dstSegment, dst,
16971720
ListReader(srcSegment, ptr, elementCount, step, dataSize, pointerCount, elementSize,
16981721
nestingLimit - 1),
@@ -1931,6 +1954,15 @@ struct WireHelpers {
19311954
goto useDefault;
19321955
}
19331956

1957+
if (wordsPerElement * (1 * ELEMENTS) == 0 * WORDS) {
1958+
// Watch out for lists of zero-sized structs, which can claim to be arbitrarily large
1959+
// without having sent actual data.
1960+
KJ_REQUIRE(amplifiedRead(segment, size * (1 * WORDS / ELEMENTS)),
1961+
"Message contains amplified list pointer.") {
1962+
goto useDefault;
1963+
}
1964+
}
1965+
19341966
if (checkElementSize) {
19351967
// If a struct list was not expected, then presumably a non-struct list was upgraded to a
19361968
// struct list. We need to manipulate the pointer to point at the first field of the
@@ -1988,14 +2020,24 @@ struct WireHelpers {
19882020
BitCount dataSize = dataBitsPerElement(ref->listRef.elementSize()) * ELEMENTS;
19892021
WirePointerCount pointerCount =
19902022
pointersPerElement(ref->listRef.elementSize()) * ELEMENTS;
2023+
ElementCount elementCount = ref->listRef.elementCount();
19912024
auto step = (dataSize + pointerCount * BITS_PER_POINTER) / ELEMENTS;
19922025

1993-
KJ_REQUIRE(boundsCheck(segment, ptr, ptr +
1994-
roundBitsUpToWords(ElementCount64(ref->listRef.elementCount()) * step)),
2026+
WordCount wordCount = roundBitsUpToWords(ElementCount64(elementCount) * step);
2027+
KJ_REQUIRE(boundsCheck(segment, ptr, ptr + wordCount),
19952028
"Message contains out-of-bounds list pointer.") {
19962029
goto useDefault;
19972030
}
19982031

2032+
if (elementSize == ElementSize::VOID) {
2033+
// Watch out for lists of void, which can claim to be arbitrarily large without having sent
2034+
// actual data.
2035+
KJ_REQUIRE(amplifiedRead(segment, elementCount * (1 * WORDS / ELEMENTS)),
2036+
"Message contains amplified list pointer.") {
2037+
goto useDefault;
2038+
}
2039+
}
2040+
19992041
if (checkElementSize) {
20002042
if (elementSize == ElementSize::BIT && expectedElementSize != ElementSize::BIT) {
20012043
KJ_FAIL_REQUIRE(
@@ -2025,7 +2067,7 @@ struct WireHelpers {
20252067
}
20262068
}
20272069

2028-
return ListReader(segment, ptr, ref->listRef.elementCount(), step,
2070+
return ListReader(segment, ptr, elementCount, step,
20292071
dataSize, pointerCount, elementSize, nestingLimit - 1);
20302072
}
20312073
}

0 commit comments

Comments
 (0)