Skip to content
Permalink
Browse files Browse the repository at this point in the history
SECURITY: Additional CPU amplification case.
Unfortunately, commit 1048706 missed a case of CPU amplification via struct lists with zero-sized elements.

See advisory: https://github.com/sandstorm-io/capnproto/blob/master/security-advisories/2015-03-05-0-c++-addl-cpu-amplification.md
  • Loading branch information
kentonv committed Mar 5, 2015
1 parent 63187bd commit 8014974
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 25 deletions.
21 changes: 14 additions & 7 deletions c++/src/capnp/encoding-test.c++
Expand Up @@ -1427,17 +1427,24 @@ TEST(Encoding, VoidListAmplification) {
}

TEST(Encoding, EmptyStructListAmplification) {
MallocMessageBuilder builder;
builder.initRoot<test::TestAnyPointer>().getAnyPointerField()
.initAs<List<test::TestEmptyStruct>>(1u << 28);
MallocMessageBuilder builder(1024);
auto listList = builder.initRoot<test::TestAnyPointer>().getAnyPointerField()
.initAs<List<List<test::TestEmptyStruct>>>(500);

for (uint i = 0; i < listList.size(); i++) {
listList.init(i, 1u << 28);
}

auto segments = builder.getSegmentsForOutput();
EXPECT_EQ(1, segments.size());
EXPECT_LT(segments[0].size(), 16); // quite small for such a big list!
ASSERT_EQ(1, segments.size());

SegmentArrayMessageReader reader(builder.getSegmentsForOutput());
auto root = reader.getRoot<test::TestAnyPointer>().getAnyPointerField();
EXPECT_NONFATAL_FAILURE(root.getAs<List<TestAllTypes>>());
auto root = reader.getRoot<test::TestAnyPointer>();
auto listListReader = root.getAnyPointerField().getAs<List<List<TestAllTypes>>>();
EXPECT_NONFATAL_FAILURE(listListReader[0]);
EXPECT_NONFATAL_FAILURE(listListReader[10]);

EXPECT_EQ(segments[0].size() - 1, root.totalSize().wordCount);
}

TEST(Encoding, Constants) {
Expand Down
42 changes: 24 additions & 18 deletions c++/src/capnp/layout.c++
Expand Up @@ -555,14 +555,16 @@ struct WireHelpers {
WordCount dataSize = elementTag->structRef.dataSize.get();
WirePointerCount pointerCount = elementTag->structRef.ptrCount.get();

word* pos = ptr + POINTER_SIZE_IN_WORDS;
uint count = elementTag->inlineCompositeListElementCount() / ELEMENTS;
for (uint i = 0; i < count; i++) {
pos += dataSize;

for (uint j = 0; j < pointerCount / POINTERS; j++) {
zeroObject(segment, reinterpret_cast<WirePointer*>(pos));
pos += POINTER_SIZE_IN_WORDS;
if (pointerCount > 0 * POINTERS) {
word* pos = ptr + POINTER_SIZE_IN_WORDS;
for (uint i = 0; i < count; i++) {
pos += dataSize;

for (uint j = 0; j < pointerCount / POINTERS; j++) {
zeroObject(segment, reinterpret_cast<WirePointer*>(pos));
pos += POINTER_SIZE_IN_WORDS;
}
}
}

Expand Down Expand Up @@ -680,8 +682,6 @@ struct WireHelpers {
return result;
}

result.wordCount += wordCount + POINTER_SIZE_IN_WORDS;

const WirePointer* elementTag = reinterpret_cast<const WirePointer*>(ptr);
ElementCount count = elementTag->inlineCompositeListElementCount();

Expand All @@ -690,23 +690,29 @@ struct WireHelpers {
return result;
}

KJ_REQUIRE(elementTag->structRef.wordSize() / ELEMENTS *
ElementCount64(count) <= wordCount,
auto actualSize = elementTag->structRef.wordSize() / ELEMENTS * ElementCount64(count);
KJ_REQUIRE(actualSize <= wordCount,
"Struct list pointer's elements overran size.") {
return result;
}

// We count the actual size rather than the claimed word count because that's what
// we'll end up with if we make a copy.
result.wordCount += actualSize + POINTER_SIZE_IN_WORDS;

WordCount dataSize = elementTag->structRef.dataSize.get();
WirePointerCount pointerCount = elementTag->structRef.ptrCount.get();

const word* pos = ptr + POINTER_SIZE_IN_WORDS;
for (uint i = 0; i < count / ELEMENTS; i++) {
pos += dataSize;
if (pointerCount > 0 * POINTERS) {
const word* pos = ptr + POINTER_SIZE_IN_WORDS;
for (uint i = 0; i < count / ELEMENTS; i++) {
pos += dataSize;

for (uint j = 0; j < pointerCount / POINTERS; j++) {
result += totalSize(segment, reinterpret_cast<const WirePointer*>(pos),
nestingLimit);
pos += POINTER_SIZE_IN_WORDS;
for (uint j = 0; j < pointerCount / POINTERS; j++) {
result += totalSize(segment, reinterpret_cast<const WirePointer*>(pos),
nestingLimit);
pos += POINTER_SIZE_IN_WORDS;
}
}
}
break;
Expand Down

0 comments on commit 8014974

Please sign in to comment.