From 229b8c63c5066d2db5cb7ff66885a568ae1e3d57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20H=C3=BCbschle?= Date: Wed, 26 Aug 2026 17:50:26 +0200 Subject: [PATCH 1/2] fix(parquet): bound schema nesting depth to prevent stack overflow A Parquet schema is a flat list of SchemaElements whose num_children fields describe the tree, so a file can nest as deep as it has elements. schema::Unflatten rebuilds that tree with a recursive lambda and no depth guard, running one native frame per level. A schema a few tens of thousands deep overflows the thread stack while the metadata is being parsed, before any consumer sees a single row: an unauthenticated remote crash on any read of an attacker-supplied Parquet file. Reject schemas nesting deeper than kMaxSchemaNestingDepth (1000) by threading a depth counter through NextNode. 1000 matches the JSON reader's kMaxNestingDepth so every untrusted-nesting path in the library fails at the same number, and it is far above any schema a real writer produces (Parquet's own 3-level LIST encoding spends 2 elements per array level). The file that motivated this is a metadata-only Parquet file (no row groups) whose schema nests 100000 groups: pre-patch it SIGSEGVs inside Unflatten; post-patch it raises "Parquet schema nesting depth exceeds the maximum of 1000". Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012wER7pGuTzevy6j4DS7hcL --- cpp/src/parquet/schema.cc | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/cpp/src/parquet/schema.cc b/cpp/src/parquet/schema.cc index 0cfa49c21c16..93b6e64ece12 100644 --- a/cpp/src/parquet/schema.cc +++ b/cpp/src/parquet/schema.cc @@ -544,6 +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; + std::unique_ptr Unflatten(const format::SchemaElement* elements, int length) { if (elements[0].num_children == 0) { if (length == 1) { @@ -560,7 +564,16 @@ std::unique_ptr Unflatten(const format::SchemaElement* elements, int lengt int pos = 0; - std::function()> NextNode = [&]() { + std::function(int)> NextNode = [&](int depth) { + // The schema is a flat list of elements whose num_children fields describe the tree, so a + // file can nest as deep as it has elements and this recursion runs one native frame per + // level. Without a cap, a schema a few tens of thousands deep overflows the thread stack + // before any consumer sees the file -- an unauthenticated remote crash on any read of an + // attacker-supplied Parquet file. Mirrors the JSON reader's kMaxNestingDepth guard. + if (depth > kMaxSchemaNestingDepth) { + throw ParquetException("Parquet schema nesting depth exceeds the maximum of " + + std::to_string(kMaxSchemaNestingDepth)); + } if (pos == length) { throw ParquetException("Malformed schema: not enough elements"); } @@ -574,13 +587,13 @@ std::unique_ptr Unflatten(const format::SchemaElement* elements, int lengt // Group node (may have 0 children, but cannot have a type) NodeVector fields; for (int i = 0; i < element.num_children; ++i) { - std::unique_ptr field = NextNode(); + std::unique_ptr field = NextNode(depth + 1); fields.push_back(NodePtr(field.release())); } return GroupNode::FromParquet(opaque_element, std::move(fields)); } }; - return NextNode(); + return NextNode(0); } std::shared_ptr FromParquet(const std::vector& schema) { From 61ade916fe338c9714d41e5e4ed9790e2e5eb951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenz=20H=C3=BCbschle?= Date: Wed, 26 Aug 2026 17:55:34 +0200 Subject: [PATCH 2/2] fix(validate): reject a negative array offset in ValidateLayout ValidateLayout checks a negative length but not a negative offset, and the two are not equivalent. Every buffer-size check in the function sizes buffers by length_plus_offset; a negative offset shrinks that, so a buffer far too small for the array satisfies all of them. ArrayData::GetValues() then hands out buffers[i]->data() + offset, a pointer before the start of the buffer, and every consumer that trusts Validate() reads out of bounds. Nothing in the columnar format permits a negative offset, but an ArrowArray arriving over the C data interface carries whatever its producer wrote: ArrayImporter::AllocateArrayData copies c_struct_->offset into ArrayData unchecked, and the importer's own buffer-size arithmetic is wrong in the same direction. Validate() is the boundary where an imported array is supposed to become trustworthy, so the check belongs here. Two lines next to the existing length check; recursion into child_data means nested arrays are covered too. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012wER7pGuTzevy6j4DS7hcL --- cpp/src/arrow/array/validate.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cpp/src/arrow/array/validate.cc b/cpp/src/arrow/array/validate.cc index c1d96375bf09..9e2a3a9132ef 100644 --- a/cpp/src/arrow/array/validate.cc +++ b/cpp/src/arrow/array/validate.cc @@ -478,6 +478,11 @@ struct ValidateArrayImpl { return Status::Invalid("Array length is negative"); } + // Shrinks length_plus_offset, so every buffer-size check below passes vacuously. + if (data.offset < 0) { + return Status::Invalid("Array offset is negative"); + } + if (layout.variadic_spec) { if (data.buffers.size() < layout.buffers.size()) { return Status::Invalid("Expected at least ", layout.buffers.size(),