diff --git a/cpp/src/arrow/array/validate.cc b/cpp/src/arrow/array/validate.cc index 9e2a3a9132ef..bfb5f4fd99ce 100644 --- a/cpp/src/arrow/array/validate.cc +++ b/cpp/src/arrow/array/validate.cc @@ -104,15 +104,23 @@ struct BoundsChecker { } }; +// Recursion limit for the nested walk below. An array can nest as deep as its type does, and +// every level costs one native frame, so an untrusted array has to be rejected before the stack +// runs out. Matches parquet's kMaxSchemaNestingDepth and the JSON reader's limit. +static constexpr int kMaxValidationNestingDepth = 256; + struct ValidateArrayImpl { const ArrayData& data; const bool full_validation; + const int depth = 0; Status Validate() { if (data.type == nullptr) { return Status::Invalid("Array type is absent"); } + RETURN_NOT_OK(CheckNestingDepth()); + // XXX should we unpack extension types here? RETURN_NOT_OK(ValidateLayout(*data.type)); @@ -454,8 +462,19 @@ struct ValidateArrayImpl { } Status Visit(const ExtensionType& type) { - // Visit storage - return ValidateWithType(*type.storage_type()); + // Visit storage. It counts as a level: extension types chain, and unlike the other nested + // types this hop keeps the same ArrayData, so nothing else would increment the depth. + ValidateArrayImpl storage_impl{data, full_validation, depth + 1}; + RETURN_NOT_OK(storage_impl.CheckNestingDepth()); + return storage_impl.ValidateWithType(*type.storage_type()); + } + + Status CheckNestingDepth() const { + if (depth > kMaxValidationNestingDepth) { + return Status::Invalid("Array nesting depth exceeds the maximum of ", + kMaxValidationNestingDepth); + } + return Status::OK(); } private: @@ -466,7 +485,7 @@ struct ValidateArrayImpl { } Status RecurseInto(const ArrayData& related_data) { - ValidateArrayImpl impl{related_data, full_validation}; + ValidateArrayImpl impl{related_data, full_validation, depth + 1}; return impl.Validate(); } diff --git a/cpp/src/arrow/json/parser.cc b/cpp/src/arrow/json/parser.cc index fa3a5ca34c47..3b994a0a2ce6 100644 --- a/cpp/src/arrow/json/parser.cc +++ b/cpp/src/arrow/json/parser.cc @@ -951,7 +951,7 @@ class HandlerBase : public BlockParser, // Maximum object/array nesting depth accepted before StartNested() rejects the input, to keep // the recursive builder finalization (RawArrayBuilder::Finish / RawBuilderSet::Finish) from // overflowing the native stack on deeply nested JSON. - static constexpr size_t kMaxNestingDepth = 1000; + static constexpr size_t kMaxNestingDepth = 256; Status status_; RawBuilderSet builder_set_; diff --git a/cpp/src/parquet/schema.cc b/cpp/src/parquet/schema.cc index 93b6e64ece12..2895b48da8a6 100644 --- a/cpp/src/parquet/schema.cc +++ b/cpp/src/parquet/schema.cc @@ -544,9 +544,10 @@ void PrimitiveNode::ToParquet(void* opaque_element) const { // ---------------------------------------------------------------------- // Schema converters -// Deepest group nesting Unflatten will build before rejecting the schema. 1000 matches the JSON -// reader's kMaxNestingDepth, so every untrusted-nesting path in the library fails at one number. -static constexpr int kMaxSchemaNestingDepth = 1000; +// Deepest group nesting Unflatten will build before rejecting the schema. Every untrusted-nesting +// path in the library shares this number; 256 is far past any real schema and leaves an order of +// magnitude of headroom under the shallowest measured overflow (~1650 frames on a 2 MB stack). +static constexpr int kMaxSchemaNestingDepth = 256; std::unique_ptr Unflatten(const format::SchemaElement* elements, int length) { if (elements[0].num_children == 0) {