From 8876101d2d16f3fc9d748d0680b5bda80775e862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20H=C3=BCbschle?= Date: Thu, 27 Aug 2026 14:15:57 +0200 Subject: [PATCH 1/4] Array::Validate(): bound nesting depth --- cpp/src/arrow/array/validate.cc | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/array/validate.cc b/cpp/src/arrow/array/validate.cc index 9e2a3a9132ef..b2db741817ac 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 = 1000; + 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(); } From 5377470cfd42c85f1053453d15d9055227f4ea47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20H=C3=BCbschle?= Date: Thu, 27 Aug 2026 14:17:26 +0200 Subject: [PATCH 2/4] fix(ipc): GetSchema() recursion depth limit --- cpp/src/arrow/ipc/dictionary.h | 3 +++ cpp/src/arrow/ipc/metadata_internal.cc | 21 +++++++++++++++------ cpp/src/arrow/ipc/metadata_internal.h | 7 +++++-- cpp/src/arrow/ipc/reader.cc | 6 ++++-- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/cpp/src/arrow/ipc/dictionary.h b/cpp/src/arrow/ipc/dictionary.h index e4287cb19747..0800e9864b0b 100644 --- a/cpp/src/arrow/ipc/dictionary.h +++ b/cpp/src/arrow/ipc/dictionary.h @@ -41,6 +41,9 @@ class FieldPosition { FieldPosition child(int index) const { return {this, index}; } + // Nesting level, counting the schema root as 0 and its top-level fields as 1. + int depth() const { return depth_; } + std::vector path() const { std::vector path(depth_); const FieldPosition* cur = this; diff --git a/cpp/src/arrow/ipc/metadata_internal.cc b/cpp/src/arrow/ipc/metadata_internal.cc index 7f2a47b06947..17b15d861108 100644 --- a/cpp/src/arrow/ipc/metadata_internal.cc +++ b/cpp/src/arrow/ipc/metadata_internal.cc @@ -862,7 +862,15 @@ class FieldToFlatbufferVisitor { }; Status FieldFromFlatbuffer(const flatbuf::Field* field, FieldPosition field_pos, - DictionaryMemo* dictionary_memo, std::shared_ptr* out) { + int max_recursion_depth, DictionaryMemo* dictionary_memo, + std::shared_ptr* out) { + // Checked on the way down: the flatbuffer verifier bounds the serialized graph, not the number + // of type levels it decodes to, and each level here costs a native frame. + if (field_pos.depth() > max_recursion_depth) { + return Status::Invalid("Schema nesting depth exceeds the maximum of ", + max_recursion_depth); + } + std::shared_ptr type; std::shared_ptr metadata; @@ -877,7 +885,8 @@ Status FieldFromFlatbuffer(const flatbuf::Field* field, FieldPosition field_pos, child_fields.resize(children->size()); for (int i = 0; i < static_cast(children->size()); ++i) { RETURN_NOT_OK(FieldFromFlatbuffer(children->Get(i), field_pos.child(i), - dictionary_memo, &child_fields[i])); + max_recursion_depth, dictionary_memo, + &child_fields[i])); } } @@ -1443,8 +1452,8 @@ Status WriteFileFooter(const Schema& schema, const std::vector& dicti // ---------------------------------------------------------------------- -Status GetSchema(const void* opaque_schema, DictionaryMemo* dictionary_memo, - std::shared_ptr* out) { +Status GetSchema(const void* opaque_schema, int max_recursion_depth, + DictionaryMemo* dictionary_memo, std::shared_ptr* out) { auto schema = static_cast(opaque_schema); CHECK_FLATBUFFERS_NOT_NULL(schema, "schema"); CHECK_FLATBUFFERS_NOT_NULL(schema->fields(), "Schema.fields"); @@ -1457,8 +1466,8 @@ Status GetSchema(const void* opaque_schema, DictionaryMemo* dictionary_memo, const flatbuf::Field* field = schema->fields()->Get(i); // XXX I don't think this check is necessary (AP) CHECK_FLATBUFFERS_NOT_NULL(field, "DictionaryEncoding.indexType"); - RETURN_NOT_OK( - FieldFromFlatbuffer(field, field_pos.child(i), dictionary_memo, &fields[i])); + RETURN_NOT_OK(FieldFromFlatbuffer(field, field_pos.child(i), max_recursion_depth, + dictionary_memo, &fields[i])); } std::shared_ptr metadata; diff --git a/cpp/src/arrow/ipc/metadata_internal.h b/cpp/src/arrow/ipc/metadata_internal.h index 2a9574d84a10..a462188d1fc6 100644 --- a/cpp/src/arrow/ipc/metadata_internal.h +++ b/cpp/src/arrow/ipc/metadata_internal.h @@ -126,9 +126,12 @@ inline std::string StringFromFlatbuffers(const flatbuffers::String* s) { // dictionary-encoded fields to a DictionaryMemo instance. May be // expensive for very large schemas if you are only interested in a // few fields +// +// Rejects schemas nested deeper than max_recursion_depth: reconstruction is recursive, and the +// flatbuffer verifier's own limit bounds the serialized graph rather than the decoded type levels. ARROW_EXPORT -Status GetSchema(const void* opaque_schema, DictionaryMemo* dictionary_memo, - std::shared_ptr* out); +Status GetSchema(const void* opaque_schema, int max_recursion_depth, + DictionaryMemo* dictionary_memo, std::shared_ptr* out); ARROW_EXPORT Status GetTensorMetadata(const Buffer& metadata, std::shared_ptr* type, diff --git a/cpp/src/arrow/ipc/reader.cc b/cpp/src/arrow/ipc/reader.cc index 5ed1f1c83c04..2bc78bd70d17 100644 --- a/cpp/src/arrow/ipc/reader.cc +++ b/cpp/src/arrow/ipc/reader.cc @@ -881,7 +881,8 @@ Status UnpackSchemaMessage(const void* opaque_schema, const IpcReadOptions& opti std::shared_ptr* schema, std::shared_ptr* out_schema, std::vector* field_inclusion_mask, bool* swap_endian) { - RETURN_NOT_OK(internal::GetSchema(opaque_schema, dictionary_memo, schema)); + RETURN_NOT_OK(internal::GetSchema(opaque_schema, options.max_recursion_depth, + dictionary_memo, schema)); // If we are selecting only certain fields, populate the inclusion mask now // for fast lookups @@ -2281,7 +2282,8 @@ Result> ReadSchema(io::InputStream* stream, Result> ReadSchema(const Message& message, DictionaryMemo* dictionary_memo) { std::shared_ptr result; - RETURN_NOT_OK(internal::GetSchema(message.header(), dictionary_memo, &result)); + RETURN_NOT_OK(internal::GetSchema(message.header(), kMaxNestingDepth, dictionary_memo, + &result)); return result; } From aed38c39ebff458f397b641b3b1f9ba75b2b01c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20H=C3=BCbschle?= Date: Thu, 27 Aug 2026 14:53:00 +0200 Subject: [PATCH 3/4] Bound depths to 256, not 1000: 1000 is too close to a stack overflow --- cpp/src/arrow/array/validate.cc | 2 +- cpp/src/arrow/json/parser.cc | 2 +- cpp/src/parquet/schema.cc | 7 ++++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/array/validate.cc b/cpp/src/arrow/array/validate.cc index b2db741817ac..bfb5f4fd99ce 100644 --- a/cpp/src/arrow/array/validate.cc +++ b/cpp/src/arrow/array/validate.cc @@ -107,7 +107,7 @@ 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 = 1000; +static constexpr int kMaxValidationNestingDepth = 256; struct ValidateArrayImpl { const ArrayData& data; 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) { From 53a812649c3fc1ae41b6e05ac87286cf345d82c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20H=C3=BCbschle?= Date: Thu, 27 Aug 2026 15:23:19 +0200 Subject: [PATCH 4/4] Revert "fix(ipc): GetSchema() recursion depth limit" This reverts commit 5377470cfd42c85f1053453d15d9055227f4ea47. --- cpp/src/arrow/ipc/dictionary.h | 3 --- cpp/src/arrow/ipc/metadata_internal.cc | 21 ++++++--------------- cpp/src/arrow/ipc/metadata_internal.h | 7 ++----- cpp/src/arrow/ipc/reader.cc | 6 ++---- 4 files changed, 10 insertions(+), 27 deletions(-) diff --git a/cpp/src/arrow/ipc/dictionary.h b/cpp/src/arrow/ipc/dictionary.h index 0800e9864b0b..e4287cb19747 100644 --- a/cpp/src/arrow/ipc/dictionary.h +++ b/cpp/src/arrow/ipc/dictionary.h @@ -41,9 +41,6 @@ class FieldPosition { FieldPosition child(int index) const { return {this, index}; } - // Nesting level, counting the schema root as 0 and its top-level fields as 1. - int depth() const { return depth_; } - std::vector path() const { std::vector path(depth_); const FieldPosition* cur = this; diff --git a/cpp/src/arrow/ipc/metadata_internal.cc b/cpp/src/arrow/ipc/metadata_internal.cc index 17b15d861108..7f2a47b06947 100644 --- a/cpp/src/arrow/ipc/metadata_internal.cc +++ b/cpp/src/arrow/ipc/metadata_internal.cc @@ -862,15 +862,7 @@ class FieldToFlatbufferVisitor { }; Status FieldFromFlatbuffer(const flatbuf::Field* field, FieldPosition field_pos, - int max_recursion_depth, DictionaryMemo* dictionary_memo, - std::shared_ptr* out) { - // Checked on the way down: the flatbuffer verifier bounds the serialized graph, not the number - // of type levels it decodes to, and each level here costs a native frame. - if (field_pos.depth() > max_recursion_depth) { - return Status::Invalid("Schema nesting depth exceeds the maximum of ", - max_recursion_depth); - } - + DictionaryMemo* dictionary_memo, std::shared_ptr* out) { std::shared_ptr type; std::shared_ptr metadata; @@ -885,8 +877,7 @@ Status FieldFromFlatbuffer(const flatbuf::Field* field, FieldPosition field_pos, child_fields.resize(children->size()); for (int i = 0; i < static_cast(children->size()); ++i) { RETURN_NOT_OK(FieldFromFlatbuffer(children->Get(i), field_pos.child(i), - max_recursion_depth, dictionary_memo, - &child_fields[i])); + dictionary_memo, &child_fields[i])); } } @@ -1452,8 +1443,8 @@ Status WriteFileFooter(const Schema& schema, const std::vector& dicti // ---------------------------------------------------------------------- -Status GetSchema(const void* opaque_schema, int max_recursion_depth, - DictionaryMemo* dictionary_memo, std::shared_ptr* out) { +Status GetSchema(const void* opaque_schema, DictionaryMemo* dictionary_memo, + std::shared_ptr* out) { auto schema = static_cast(opaque_schema); CHECK_FLATBUFFERS_NOT_NULL(schema, "schema"); CHECK_FLATBUFFERS_NOT_NULL(schema->fields(), "Schema.fields"); @@ -1466,8 +1457,8 @@ Status GetSchema(const void* opaque_schema, int max_recursion_depth, const flatbuf::Field* field = schema->fields()->Get(i); // XXX I don't think this check is necessary (AP) CHECK_FLATBUFFERS_NOT_NULL(field, "DictionaryEncoding.indexType"); - RETURN_NOT_OK(FieldFromFlatbuffer(field, field_pos.child(i), max_recursion_depth, - dictionary_memo, &fields[i])); + RETURN_NOT_OK( + FieldFromFlatbuffer(field, field_pos.child(i), dictionary_memo, &fields[i])); } std::shared_ptr metadata; diff --git a/cpp/src/arrow/ipc/metadata_internal.h b/cpp/src/arrow/ipc/metadata_internal.h index a462188d1fc6..2a9574d84a10 100644 --- a/cpp/src/arrow/ipc/metadata_internal.h +++ b/cpp/src/arrow/ipc/metadata_internal.h @@ -126,12 +126,9 @@ inline std::string StringFromFlatbuffers(const flatbuffers::String* s) { // dictionary-encoded fields to a DictionaryMemo instance. May be // expensive for very large schemas if you are only interested in a // few fields -// -// Rejects schemas nested deeper than max_recursion_depth: reconstruction is recursive, and the -// flatbuffer verifier's own limit bounds the serialized graph rather than the decoded type levels. ARROW_EXPORT -Status GetSchema(const void* opaque_schema, int max_recursion_depth, - DictionaryMemo* dictionary_memo, std::shared_ptr* out); +Status GetSchema(const void* opaque_schema, DictionaryMemo* dictionary_memo, + std::shared_ptr* out); ARROW_EXPORT Status GetTensorMetadata(const Buffer& metadata, std::shared_ptr* type, diff --git a/cpp/src/arrow/ipc/reader.cc b/cpp/src/arrow/ipc/reader.cc index 2bc78bd70d17..5ed1f1c83c04 100644 --- a/cpp/src/arrow/ipc/reader.cc +++ b/cpp/src/arrow/ipc/reader.cc @@ -881,8 +881,7 @@ Status UnpackSchemaMessage(const void* opaque_schema, const IpcReadOptions& opti std::shared_ptr* schema, std::shared_ptr* out_schema, std::vector* field_inclusion_mask, bool* swap_endian) { - RETURN_NOT_OK(internal::GetSchema(opaque_schema, options.max_recursion_depth, - dictionary_memo, schema)); + RETURN_NOT_OK(internal::GetSchema(opaque_schema, dictionary_memo, schema)); // If we are selecting only certain fields, populate the inclusion mask now // for fast lookups @@ -2282,8 +2281,7 @@ Result> ReadSchema(io::InputStream* stream, Result> ReadSchema(const Message& message, DictionaryMemo* dictionary_memo) { std::shared_ptr result; - RETURN_NOT_OK(internal::GetSchema(message.header(), kMaxNestingDepth, dictionary_memo, - &result)); + RETURN_NOT_OK(internal::GetSchema(message.header(), dictionary_memo, &result)); return result; }