Limit recursion depth during parsing.
Fuzzing identified a way to crash (without security risk)
the libcppbor parser, by providing as input CBOR that
encodes a structure thousands of layers deep. Since the
parser uses recursion to handle structure nesting, and each
layer adds three largish stack frames, it's pretty easy to
exhaust the stack.
At some point in the future I may refactor to make the
parser non-recursive, but for now this CL just limits the
parseable structure depth to 1000, which seems very unlikely
to cause problems with any real CBOR structure, but should
prevent stack exhaustion except in environments with
unusually-small stacks.
Bug: 319143145
Bug: 330863728
Bug: 184903388
Test: clusterfuzz test cases from referenced bugs
Change-Id: I5ace86a26f14e33da58369ab415df34866c8fcf0
diff --git a/src/cppbor_parse.cpp b/src/cppbor_parse.cpp
index 2614c4e..c3fa070 100644
--- a/src/cppbor_parse.cpp
+++ b/src/cppbor_parse.cpp
@@ -33,6 +33,8 @@
namespace {
+const unsigned kMaxParseDepth = 1000;
+
std::string insufficientLengthString(size_t bytesNeeded, size_t bytesAvail,
const std::string& type) {
char buf[1024];
@@ -58,7 +60,8 @@
}
std::tuple<const uint8_t*, ParseClient*> parseRecursively(const uint8_t* begin, const uint8_t* end,
- bool emitViews, ParseClient* parseClient);
+ bool emitViews, ParseClient* parseClient,
+ unsigned depth);
std::tuple<const uint8_t*, ParseClient*> handleUint(uint64_t value, const uint8_t* hdrBegin,
const uint8_t* hdrEnd,
@@ -205,16 +208,15 @@
std::tuple<const uint8_t*, ParseClient*> handleEntries(size_t entryCount, const uint8_t* hdrBegin,
const uint8_t* pos, const uint8_t* end,
- const std::string& typeName,
- bool emitViews,
- ParseClient* parseClient) {
+ const std::string& typeName, bool emitViews,
+ ParseClient* parseClient, unsigned depth) {
while (entryCount > 0) {
--entryCount;
if (pos == end) {
parseClient->error(hdrBegin, "Not enough entries for " + typeName + ".");
return {hdrBegin, nullptr /* end parsing */};
}
- std::tie(pos, parseClient) = parseRecursively(pos, end, emitViews, parseClient);
+ std::tie(pos, parseClient) = parseRecursively(pos, end, emitViews, parseClient, depth + 1);
if (!parseClient) return {hdrBegin, nullptr};
}
return {pos, parseClient};
@@ -222,22 +224,23 @@
std::tuple<const uint8_t*, ParseClient*> handleCompound(
std::unique_ptr<Item> item, uint64_t entryCount, const uint8_t* hdrBegin,
- const uint8_t* valueBegin, const uint8_t* end, const std::string& typeName,
- bool emitViews, ParseClient* parseClient) {
+ const uint8_t* valueBegin, const uint8_t* end, const std::string& typeName, bool emitViews,
+ ParseClient* parseClient, unsigned depth) {
parseClient =
parseClient->item(item, hdrBegin, valueBegin, valueBegin /* don't know the end yet */);
if (!parseClient) return {hdrBegin, nullptr};
const uint8_t* pos;
- std::tie(pos, parseClient) =
- handleEntries(entryCount, hdrBegin, valueBegin, end, typeName, emitViews, parseClient);
+ std::tie(pos, parseClient) = handleEntries(entryCount, hdrBegin, valueBegin, end, typeName,
+ emitViews, parseClient, depth);
if (!parseClient) return {hdrBegin, nullptr};
return {pos, parseClient->itemEnd(item, hdrBegin, valueBegin, pos)};
}
std::tuple<const uint8_t*, ParseClient*> parseRecursively(const uint8_t* begin, const uint8_t* end,
- bool emitViews, ParseClient* parseClient) {
+ bool emitViews, ParseClient* parseClient,
+ unsigned depth) {
if (begin == end) {
parseClient->error(
begin,
@@ -245,6 +248,15 @@
return {begin, nullptr};
}
+ // Limit recursion depth to avoid overflowing the stack.
+ if (depth > kMaxParseDepth) {
+ parseClient->error(begin,
+ "Max depth reached. Cannot parse CBOR structures with more "
+ "than " +
+ std::to_string(kMaxParseDepth) + " levels.");
+ return {begin, nullptr};
+ }
+
const uint8_t* pos = begin;
MajorType type = static_cast<MajorType>(*pos & 0xE0);
@@ -309,15 +321,15 @@
case ARRAY:
return handleCompound(std::make_unique<IncompleteArray>(addlData), addlData, begin, pos,
- end, "array", emitViews, parseClient);
+ end, "array", emitViews, parseClient, depth);
case MAP:
return handleCompound(std::make_unique<IncompleteMap>(addlData), addlData * 2, begin,
- pos, end, "map", emitViews, parseClient);
+ pos, end, "map", emitViews, parseClient, depth);
case SEMANTIC:
return handleCompound(std::make_unique<IncompleteSemanticTag>(addlData), 1, begin, pos,
- end, "semantic", emitViews, parseClient);
+ end, "semantic", emitViews, parseClient, depth);
case SIMPLE:
switch (addlData) {
@@ -402,7 +414,7 @@
} // anonymous namespace
void parse(const uint8_t* begin, const uint8_t* end, ParseClient* parseClient) {
- parseRecursively(begin, end, false, parseClient);
+ parseRecursively(begin, end, false, parseClient, 0);
}
std::tuple<std::unique_ptr<Item> /* result */, const uint8_t* /* newPos */,
@@ -414,7 +426,7 @@
}
void parseWithViews(const uint8_t* begin, const uint8_t* end, ParseClient* parseClient) {
- parseRecursively(begin, end, true, parseClient);
+ parseRecursively(begin, end, true, parseClient, 0);
}
std::tuple<std::unique_ptr<Item> /* result */, const uint8_t* /* newPos */,
diff --git a/tests/cppbor_test.cpp b/tests/cppbor_test.cpp
index a75acc8..fadcaa9 100644
--- a/tests/cppbor_test.cpp
+++ b/tests/cppbor_test.cpp
@@ -33,6 +33,7 @@
using ::testing::IsNull;
using ::testing::NotNull;
using ::testing::Return;
+using ::testing::StartsWith;
using ::testing::Unused;
string hexDump(const string& str) {
@@ -1579,6 +1580,30 @@
parseWithViews(encoded.data(), encoded.data() + encoded.size(), &mpc);
}
+TEST(StreamParseTest, AllowDepth1000) {
+ std::vector<uint8_t> data(/* count */ 1000, /* value = array with one entry */ 0x81);
+ data.push_back(0);
+
+ MockParseClient mpc;
+ EXPECT_CALL(mpc, item).Times(1001).WillRepeatedly(Return(&mpc));
+ EXPECT_CALL(mpc, itemEnd).Times(1000).WillRepeatedly(Return(&mpc));
+ EXPECT_CALL(mpc, error(_, _)).Times(0);
+
+ parse(data.data(), data.data() + data.size(), &mpc);
+}
+
+TEST(StreamParseTest, DisallowDepth1001) {
+ std::vector<uint8_t> data(/* count */ 1001, /* value = array with one entry */ 0x81);
+ data.push_back(0);
+
+ MockParseClient mpc;
+ EXPECT_CALL(mpc, item).Times(1001).WillRepeatedly(Return(&mpc));
+ EXPECT_CALL(mpc, itemEnd).Times(0);
+ EXPECT_CALL(mpc, error(_, StartsWith("Max depth reached"))).Times(1);
+
+ parse(data.data(), data.data() + data.size(), &mpc);
+}
+
TEST(FullParserTest, Uint) {
Uint val(10);
