Limit CBOR deepness (#7838)

Author: maxtropets

CHANGELOG.md

@@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [7.0.1]
+
+[7.0.1]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.1
+
+### Fixed
+
+- Fixed a bug in the CBOR parser where a deeply nested payload could cause a node to crash. That could be exploited, for instance, to cause a DoS on publicly avaliable COSE-authenticated endpoints (#7838).
+
 ## [7.0.0]
 
 [7.0.0]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.0

doc/build_apps/migration_6_x_to_7_0.rst

@@ -38,3 +38,5 @@ Version Live Compatibility
 When upgrading CCF services from one major version to the next, our usual recommendation is to upgrade from ``N.latest`` to ``N+1.0.0``. Interoperation between other versions is not guaranteed.
 
 .. note:: For upgrades from 6.x to 7.0 specifically, a minimum version of 6.0.21 is required before upgrading. While upgrading from the latest 6.x release is recommended for the best experience, 6.0.21 is the minimum supported version that ensures proper snapshot compatibility and consistent chunk sizes in 7.0.
+
+Update from the latest `6.x` straight to `7.0.1` intead of `7.0.0` is highly recommended due to (#7838). Compatibility is preserved.

python/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "ccf"
-version = "7.0.0"
+version = "7.0.1"
 authors = [
   { name="CCF Team", email="CCF-Sec@microsoft.com" },
 ]

src/crypto/cbor.cpp

@@ -81,7 +81,7 @@ namespace
     std::list<std::vector<cbor_raw>> arrays;
     std::list<std::vector<cbor_map_entry>> maps;
   };
-  Value consume(cbor_nondet_t cbor);
+  Value consume(cbor_nondet_t cbor, size_t depth, size_t max_depth);
 
   void print_indent(std::ostringstream& os, size_t indent)
   {
@@ -129,7 +129,7 @@ namespace
     return std::make_shared<ValueImpl>(value);
   }
 
-  Value consume_array(cbor_nondet_t cbor)
+  Value consume_array(cbor_nondet_t cbor, size_t depth, size_t max_depth)
   {
     cbor_nondet_array_iterator_t iter;
     if (!cbor_nondet_array_iterator_start(cbor, &iter))
@@ -147,12 +147,12 @@ namespace
         throw CBORDecodeError(
           Error::DECODE_FAILED, "Failed to get next array item");
       }
-      array.items.push_back(consume(item));
+      array.items.push_back(consume(item, depth + 1, max_depth));
     }
     return std::make_shared<ValueImpl>(std::move(array));
   }
 
-  Value consume_map(cbor_nondet_t cbor)
+  Value consume_map(cbor_nondet_t cbor, size_t depth, size_t max_depth)
   {
     cbor_map_iterator iter;
     if (!cbor_nondet_map_iterator_start(cbor, &iter))
@@ -171,12 +171,14 @@ namespace
         throw CBORDecodeError(
           Error::DECODE_FAILED, "Failed to get next map entry");
       }
-      map.items.emplace_back(consume(key_raw), consume(value_raw));
+      map.items.emplace_back(
+        consume(key_raw, depth + 1, max_depth),
+        consume(value_raw, depth + 1, max_depth));
     }
     return std::make_shared<ValueImpl>(std::move(map));
   }
 
-  Value consume_tagged(cbor_nondet_t cbor)
+  Value consume_tagged(cbor_nondet_t cbor, size_t depth, size_t max_depth)
   {
     uint64_t tag = 0;
     cbor_nondet_t payload;
@@ -188,7 +190,7 @@ namespace
 
     Tagged tagged;
     tagged.tag = tag;
-    tagged.item = consume(payload);
+    tagged.item = consume(payload, depth + 1, max_depth);
     return std::make_shared<ValueImpl>(std::move(tagged));
   }
 
@@ -206,8 +208,15 @@ namespace
     return std::make_shared<ValueImpl>(value);
   }
 
-  Value consume(cbor_nondet_t cbor)
+  Value consume(cbor_nondet_t cbor, size_t depth, size_t max_depth)
   {
+    if (depth > max_depth)
+    {
+      throw CBORDecodeError(
+        Error::DECODE_FAILED,
+        fmt::format("Maximum CBOR nesting depth ({}) exceeded", max_depth));
+    }
+
     const auto mt = cbor_nondet_major_type(cbor);
     switch (mt)
     {
@@ -219,11 +228,11 @@ namespace
       case CBOR_MAJOR_TYPE_TEXT_STRING:
         return consume_text_string(cbor);
       case CBOR_MAJOR_TYPE_ARRAY:
-        return consume_array(cbor);
+        return consume_array(cbor, depth, max_depth);
       case CBOR_MAJOR_TYPE_MAP:
-        return consume_map(cbor);
+        return consume_map(cbor, depth, max_depth);
       case CBOR_MAJOR_TYPE_TAGGED:
-        return consume_tagged(cbor);
+        return consume_tagged(cbor, depth, max_depth);
       case CBOR_MAJOR_TYPE_SIMPLE_VALUE:
         return consume_simple(cbor);
       default:
@@ -249,7 +258,8 @@ namespace
     }
   }
 
-  cbor_raw to_raw_cbor(const Value& value, CborRawArena& arena);
+  cbor_raw to_raw_cbor(
+    const Value& value, CborRawArena& arena, size_t depth, size_t max_depth);
 
   cbor_raw to_raw_signed(const Signed& v)
   {
@@ -295,10 +305,11 @@ namespace
     return result;
   }
 
-  cbor_raw to_raw_tagged(const Tagged& v, CborRawArena& arena)
+  cbor_raw to_raw_tagged(
+    const Tagged& v, CborRawArena& arena, size_t depth, size_t max_depth)
   {
     cbor_raw result;
-    arena.push(to_raw_cbor(v.item, arena));
+    arena.push(to_raw_cbor(v.item, arena, depth + 1, max_depth));
     if (!cbor_nondet_mk_tagged(v.tag, arena.single(), &result))
     {
       throw CBOREncodeError(
@@ -308,14 +319,15 @@ namespace
     return result;
   }
 
-  cbor_raw to_raw_array(const Array& v, CborRawArena& arena)
+  cbor_raw to_raw_array(
+    const Array& v, CborRawArena& arena, size_t depth, size_t max_depth)
   {
     cbor_raw result;
     std::vector<cbor_raw> items;
     items.reserve(v.items.size());
     for (const auto& item : v.items)
     {
-      items.push_back(to_raw_cbor(item, arena));
+      items.push_back(to_raw_cbor(item, arena, depth + 1, max_depth));
     }
 
     size_t arr_size = items.size();
@@ -337,16 +349,17 @@ namespace
     return result;
   }
 
-  cbor_raw to_raw_map(const Map& v, CborRawArena& arena)
+  cbor_raw to_raw_map(
+    const Map& v, CborRawArena& arena, size_t depth, size_t max_depth)
   {
     cbor_raw result;
 
     std::vector<cbor_map_entry> entries;
     entries.reserve(v.items.size());
     for (const auto& [key, value] : v.items)
     {
-      auto cbor_key = to_raw_cbor(key, arena);
-      auto cbor_value = to_raw_cbor(value, arena);
+      auto cbor_key = to_raw_cbor(key, arena, depth + 1, max_depth);
+      auto cbor_value = to_raw_cbor(value, arena, depth + 1, max_depth);
       entries.push_back(cbor_nondet_mk_map_entry(cbor_key, cbor_value));
     }
 
@@ -369,8 +382,16 @@ namespace
     return result;
   }
 
-  cbor_raw to_raw_cbor(const Value& value, CborRawArena& arena)
+  cbor_raw to_raw_cbor(
+    const Value& value, CborRawArena& arena, size_t depth, size_t max_depth)
   {
+    if (depth > max_depth)
+    {
+      throw CBOREncodeError(
+        Error::ENCODE_FAILED,
+        fmt::format("Maximum CBOR nesting depth ({}) exceeded", max_depth));
+    }
+
     return std::visit(
       [&](const auto& v) {
         using T = std::decay_t<decltype(v)>;
@@ -392,15 +413,15 @@ namespace
         }
         if constexpr (std::is_same_v<T, Tagged>)
         {
-          return to_raw_tagged(v, arena);
+          return to_raw_tagged(v, arena, depth, max_depth);
         }
         if constexpr (std::is_same_v<T, Array>)
         {
-          return to_raw_array(v, arena);
+          return to_raw_array(v, arena, depth, max_depth);
         }
         if constexpr (std::is_same_v<T, Map>)
         {
-          return to_raw_map(v, arena);
+          return to_raw_map(v, arena, depth, max_depth);
         }
       },
       value->value);
@@ -536,7 +557,7 @@ namespace ccf::cbor
     return std::make_shared<ValueImpl>(Map{.items = std::move(data)});
   }
 
-  Value parse(std::span<const uint8_t> raw)
+  Value parse(std::span<const uint8_t> raw, size_t max_depth)
   {
     cbor_nondet_t cbor;
     const bool check_map_key_bound = false;
@@ -561,13 +582,13 @@ namespace ccf::cbor
         fmt::format("Trailing {} byte(s) after CBOR item", cbor_parse_size));
     }
 
-    return consume(cbor);
+    return consume(cbor, 0, max_depth);
   }
 
-  std::vector<uint8_t> serialize(const Value& value)
+  std::vector<uint8_t> serialize(const Value& value, size_t max_depth)
   {
     CborRawArena arena{};
-    auto raw = to_raw_cbor(value, arena);
+    auto raw = to_raw_cbor(value, arena, 0, max_depth);
     const auto expected_size =
       cbor_nondet_size(raw, std::numeric_limits<size_t>::max());
 

src/crypto/cbor.h

@@ -113,8 +113,8 @@ namespace ccf::cbor
   Value make_array(std::vector<Value>&& data);
   Value make_map(std::vector<MapItem>&& data);
 
-  Value parse(std::span<const uint8_t> raw);
-  std::vector<uint8_t> serialize(const Value& value);
+  Value parse(std::span<const uint8_t> raw, size_t max_depth = 16);
+  std::vector<uint8_t> serialize(const Value& value, size_t max_depth = 16);
 
   std::string to_string(const Value& value);
   bool simple_to_boolean(const Simple& value);

src/crypto/test/cbor.cpp

@@ -1739,3 +1739,42 @@ TEST_CASE("CBOR: trailing bytes rejected")
   auto array_trailing = ccf::ds::from_hex("82010203");
   REQUIRE_THROWS_AS(parse(array_trailing), CBORDecodeError);
 }
+
+TEST_CASE("CBOR: parse max depth")
+{
+  // depth 1: [42] -- should pass at max_depth=2
+  auto depth1 = ccf::ds::from_hex("81182a");
+  REQUIRE_NOTHROW(parse(depth1, 2));
+
+  // depth 2: [[42]] -- should pass at max_depth=2
+  auto depth2 = ccf::ds::from_hex("8181182a");
+  REQUIRE_NOTHROW(parse(depth2, 2));
+
+  // depth 3: [[[42]]] -- should fail at max_depth=2
+  auto depth3 = ccf::ds::from_hex("818181182a");
+  REQUIRE_THROWS_AS(parse(depth3, 2), CBORDecodeError);
+
+  // map depth 3: {1: {1: {1: 42}}} -- should fail at max_depth=2
+  auto map_depth3 = ccf::ds::from_hex("a101a101a101182a");
+  REQUIRE_THROWS_AS(parse(map_depth3, 2), CBORDecodeError);
+
+  // map depth 2: {1: {1: 42}} -- should pass at max_depth=2
+  auto map_depth2 = ccf::ds::from_hex("a101a101182a");
+  REQUIRE_NOTHROW(parse(map_depth2, 2));
+}
+
+TEST_CASE("CBOR: serialize max depth")
+{
+  // depth 1: [42] -- should pass at max_depth=1,2
+  auto shallow = make_array({make_signed(42)});
+  REQUIRE_NOTHROW(serialize(shallow, 1));
+  REQUIRE_NOTHROW(serialize(shallow, 2));
+
+  // Build 3 levels: [[[42]]]
+  auto deep = make_array({make_array({make_array({make_signed(42)})})});
+  REQUIRE_THROWS_AS(serialize(deep, 2), CBOREncodeError);
+
+  // Same deep tree passes at higher limit
+  REQUIRE_NOTHROW(serialize(deep, 3));
+  REQUIRE_NOTHROW(serialize(deep, 4));
+}