Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions cpp/src/arrow/array/validate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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:
Expand All @@ -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();
}

Expand Down
3 changes: 3 additions & 0 deletions cpp/src/arrow/ipc/dictionary.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<int> path() const {
std::vector<int> path(depth_);
const FieldPosition* cur = this;
Expand Down
21 changes: 15 additions & 6 deletions cpp/src/arrow/ipc/metadata_internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,15 @@ class FieldToFlatbufferVisitor {
};

Status FieldFromFlatbuffer(const flatbuf::Field* field, FieldPosition field_pos,
DictionaryMemo* dictionary_memo, std::shared_ptr<Field>* out) {
int max_recursion_depth, DictionaryMemo* dictionary_memo,
std::shared_ptr<Field>* 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<DataType> type;

std::shared_ptr<KeyValueMetadata> metadata;
Expand All @@ -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<int>(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]));
}
}

Expand Down Expand Up @@ -1443,8 +1452,8 @@ Status WriteFileFooter(const Schema& schema, const std::vector<FileBlock>& dicti

// ----------------------------------------------------------------------

Status GetSchema(const void* opaque_schema, DictionaryMemo* dictionary_memo,
std::shared_ptr<Schema>* out) {
Status GetSchema(const void* opaque_schema, int max_recursion_depth,
DictionaryMemo* dictionary_memo, std::shared_ptr<Schema>* out) {
auto schema = static_cast<const flatbuf::Schema*>(opaque_schema);
CHECK_FLATBUFFERS_NOT_NULL(schema, "schema");
CHECK_FLATBUFFERS_NOT_NULL(schema->fields(), "Schema.fields");
Expand All @@ -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<KeyValueMetadata> metadata;
Expand Down
7 changes: 5 additions & 2 deletions cpp/src/arrow/ipc/metadata_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<Schema>* out);
Status GetSchema(const void* opaque_schema, int max_recursion_depth,
DictionaryMemo* dictionary_memo, std::shared_ptr<Schema>* out);

ARROW_EXPORT
Status GetTensorMetadata(const Buffer& metadata, std::shared_ptr<DataType>* type,
Expand Down
6 changes: 4 additions & 2 deletions cpp/src/arrow/ipc/reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,8 @@ Status UnpackSchemaMessage(const void* opaque_schema, const IpcReadOptions& opti
std::shared_ptr<Schema>* schema,
std::shared_ptr<Schema>* out_schema,
std::vector<bool>* 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
Expand Down Expand Up @@ -2281,7 +2282,8 @@ Result<std::shared_ptr<Schema>> ReadSchema(io::InputStream* stream,
Result<std::shared_ptr<Schema>> ReadSchema(const Message& message,
DictionaryMemo* dictionary_memo) {
std::shared_ptr<Schema> result;
RETURN_NOT_OK(internal::GetSchema(message.header(), dictionary_memo, &result));
RETURN_NOT_OK(internal::GetSchema(message.header(), kMaxNestingDepth, dictionary_memo,
&result));
return result;
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/json/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
7 changes: 4 additions & 3 deletions cpp/src/parquet/schema.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<Node> Unflatten(const format::SchemaElement* elements, int length) {
if (elements[0].num_children == 0) {
Expand Down