Skip to content

fix(parquet/variant): validate metadata offset tables - #1063

Open
fallintoplace wants to merge 2 commits into
apache:mainfrom
fallintoplace:fix/variant-metadata-offset-validation
Open

fix(parquet/variant): validate metadata offset tables#1063
fallintoplace wants to merge 2 commits into
apache:mainfrom
fallintoplace:fix/variant-metadata-offset-validation

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What changed

Validate the complete Variant metadata offset table before allocating the key slice or reading offsets. The parser now also rejects a nonzero initial offset, decreasing offsets, and string offsets beyond the input.

Why

The previous size check omitted the final offset entry. Truncated input could therefore reach an out-of-bounds slice, while decreasing offsets could underflow during key-size calculation. A large untrusted dictionary count was also used to allocate before checking whether the offset table could fit in the input.

Malformed metadata now consistently returns an error wrapping ErrInvalidMetadata.

Testing

  • go test ./parquet/variant

@fallintoplace
fallintoplace requested a review from zeroshade as a code owner July 29, 2026 21:35

@serramatutu serramatutu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran the test locally before and after the fix and it all passes. LGTM to me except for a the small mistake >= instead of > when checking the range.

return fmt.Errorf("%w: offset out of range: %d > %d",
ErrInvalidMetadata, (dictSize+hdrSizeBytes)*uint32(offsetSz), len(m.data))
valuesStart := uint64(hdrSizeBytes) + (uint64(dictSize)+2)*uint64(offsetSz)
if valuesStart > uint64(len(m.data)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be >=?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valuesStart == len(m.data) is valid when the offset table ends exactly at the input boundary and the dictionary’s strings contain zero total bytes.

Changing > to >= would incorrectly reject that valid encoding. The subsequent check already rejects any nonzero final offset because valuesStart + end would exceed the input length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants