diff --git a/src/dataencode.cpp b/src/dataencode.cpp index 6f6b99c..65dc19f 100644 --- a/src/dataencode.cpp +++ b/src/dataencode.cpp @@ -66,9 +66,11 @@ void to_wire(Buffer& buf, const FieldDesc* cur) } } +static constexpr unsigned maxNestingDepth = 20u; + void from_wire(Buffer& buf, std::vector& descs, TypeStore& cache, unsigned depth) { - if(!buf.good() || depth>20) { + if(!buf.good() || depth>maxNestingDepth) { buf.fault(__FILE__, __LINE__); return; } @@ -448,8 +450,12 @@ T from_wire_as(Buffer& buf) } static -void from_wire_field(Buffer& buf, TypeStore& ctxt, const FieldDesc* desc, const std::shared_ptr& store) +void from_wire_field(Buffer& buf, TypeStore& ctxt, const FieldDesc* desc, const std::shared_ptr& store, unsigned depth) { + if(depth>maxNestingDepth) { + buf.fault(__FILE__, __LINE__); + return; + } switch(store->code) { case StoreType::Null: switch(desc->code.code) { @@ -459,7 +465,7 @@ void from_wire_field(Buffer& buf, TypeStore& ctxt, const FieldDesc* desc, const auto cdesc = desc + off; std::shared_ptr cstore(store, store.get()+off); // TODO avoid shared_ptr/aliasing here if(cdesc->code!=TypeCode::Struct) { - from_wire_field(buf, ctxt, cdesc, cstore); + from_wire_field(buf, ctxt, cdesc, cstore, depth+1); cstore->valid = true; } } @@ -530,7 +536,7 @@ void from_wire_field(Buffer& buf, TypeStore& ctxt, const FieldDesc* desc, const &desc->members[desc->miter[select.index()].second]); // alias fld = Value::Helper::build(stype, store, desc); - from_wire_full(buf, ctxt, fld); + from_wire_full(buf, ctxt, fld, depth+1); return; } else { // invalid selection @@ -542,7 +548,7 @@ void from_wire_field(Buffer& buf, TypeStore& ctxt, const FieldDesc* desc, const case TypeCode::Any: { auto descs(std::make_shared>()); - from_wire(buf, *descs, ctxt); + from_wire(buf, *descs, ctxt, depth+1); if(!buf.good()) return; @@ -554,7 +560,7 @@ void from_wire_field(Buffer& buf, TypeStore& ctxt, const FieldDesc* desc, const std::shared_ptr stype(descs, descs->data()); // alias fld = Value::Helper::build(stype); - from_wire_full(buf, ctxt, fld); + from_wire_full(buf, ctxt, fld, depth+1); return; } @@ -614,7 +620,7 @@ void from_wire_field(Buffer& buf, TypeStore& ctxt, const FieldDesc* desc, const if(from_wire_as(buf)!=0) { // strictly 1 or 0 elem = Value::Helper::build(etype, store, desc); - from_wire_full(buf, ctxt, elem); + from_wire_full(buf, ctxt, elem, depth+1); } } @@ -640,7 +646,7 @@ void from_wire_field(Buffer& buf, TypeStore& ctxt, const FieldDesc* desc, const &cdesc->members[cdesc->miter[select.index()].second]); // alias elem = Value::Helper::build(stype, store, desc); - from_wire_full(buf, ctxt, elem); + from_wire_full(buf, ctxt, elem, depth+1); } else { // invalid selector @@ -662,7 +668,7 @@ void from_wire_field(Buffer& buf, TypeStore& ctxt, const FieldDesc* desc, const if(from_wire_as(buf)!=0) { // strictly 1 or 0 auto descs(std::make_shared>()); - from_wire(buf, *descs, ctxt); + from_wire(buf, *descs, ctxt, depth+1); if(!buf.good()) return; @@ -671,7 +677,7 @@ void from_wire_field(Buffer& buf, TypeStore& ctxt, const FieldDesc* desc, const std::shared_ptr stype(descs, descs->data()); // alias elem = Value::Helper::build(stype, store, desc); - from_wire_full(buf, ctxt, elem); + from_wire_full(buf, ctxt, elem, depth+1); } } } @@ -689,11 +695,11 @@ void from_wire_field(Buffer& buf, TypeStore& ctxt, const FieldDesc* desc, const buf.fault(__FILE__, __LINE__); } -void from_wire_full(Buffer& buf, TypeStore& ctxt, Value& val) +void from_wire_full(Buffer& buf, TypeStore& ctxt, Value& val, unsigned depth) { assert(!!val); - from_wire_field(buf, ctxt, Value::Helper::desc(val), Value::Helper::store(val)); + from_wire_field(buf, ctxt, Value::Helper::desc(val), Value::Helper::store(val), depth); } void from_wire_valid(Buffer& buf, TypeStore& ctxt, Value& val) @@ -720,7 +726,7 @@ void from_wire_valid(Buffer& buf, TypeStore& ctxt, Value& val) { std::shared_ptr cstore(store, store.get()+bit); auto cdesc = desc + bit; - from_wire_field(buf, ctxt, cdesc, cstore); + from_wire_field(buf, ctxt, cdesc, cstore, 0); cstore->valid = true; bit = valid.findSet(bit + cdesc->size()); } diff --git a/src/dataimpl.h b/src/dataimpl.h index 7e455ad..d67a6f9 100644 --- a/src/dataimpl.h +++ b/src/dataimpl.h @@ -179,7 +179,7 @@ void from_wire_type(Buffer& buf, TypeStore& ctxt, Value& val); //! deserialize full Value PVXS_API -void from_wire_full(Buffer& buf, TypeStore& ctxt, Value& val); +void from_wire_full(Buffer& buf, TypeStore& ctxt, Value& val, unsigned depth=0); //! deserialize BitMask and partial Value PVXS_API diff --git a/test/testxcode.cpp b/test/testxcode.cpp index 86222d2..296d336 100644 --- a/test/testxcode.cpp +++ b/test/testxcode.cpp @@ -1176,6 +1176,25 @@ void testRegressBadBitMask() } } +// A run of TypeCode::Any (0x82) leaf bytes encodes an Any value whose content is +// itself an Any, repeated. The value decoder must be bounded by its depth guard +// and fault, instead of recursing once per byte (from_wire_field -> from_wire_full +// -> from_wire_field -> ...) and exhausting the thread stack. Reaching the +// assertion at all is the regression: without the bound this overflows the stack +// and crashes the process (SIGSEGV is not a std::exception, so it is uncatchable). +void testRegressDeepAny() +{ + testDiag("%s", __func__); + + std::vector msg(1000000u, 0x82u); // 0x82 == TypeCode::Any + + TypeStore ctxt; + Value val; + FixedBuf buf(false, msg); + from_wire_type_value(buf, ctxt, val); + testFalse(buf.good())<<" deeply-nested Any must fault, not recurse without bound"; +} + // test the common case for a pvRequest of caching an empty Struct void testEmptyRequest() { @@ -1256,7 +1275,7 @@ void testPartialXCode() MAIN(testxcode) { - testPlan(150); + testPlan(151); testSetup(); testDeserializeString(); testSerialize1(); @@ -1272,6 +1291,7 @@ MAIN(testxcode) testRegressRedundantBitMask(); testRegressCNEN(); testRegressBadBitMask(); + testRegressDeepAny(); testBadFieldName(); testEmptyRequest(); testPartialXCode();