This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-go.git
The following commit(s) were added to refs/heads/main by this push:
new 27952c74 fix(parquet/variant): prevent metadata offset cursor overflow
(#1290)
27952c74 is described below
commit 27952c740323854874284c355251d3c17c29e648
Author: matanper <[email protected]>
AuthorDate: Tue Sep 8 20:10:02 2026 +0300
fix(parquet/variant): prevent metadata offset cursor overflow (#1290)
### Rationale for this change
Reading valid VARIANT metadata with 126 dictionary keys and two-byte
offsets panics with `slice bounds out of range [255:1]`. The metadata
offset cursor inherits `uint8` from `offsetSz`; both advancing the
cursor and computing the slice end can wrap at byte 256. Three- and
four-byte offset tables hit the same problem with even fewer keys.
This also affects current main after #1063: validating the complete
offset table does not prevent the cursor arithmetic from overflowing
while reading that valid table.
### What changes are included in this PR?
Use an `int` offset width and cursor for indexing the metadata buffer.
Keep the wire-format width and existing metadata validation unchanged.
Add synthetic valid-metadata tests immediately before, at, and after the
failure boundary for two-, three-, and four-byte offsets, plus larger
dictionaries. Check every decoded dictionary key. No external test data
is required.
### Are these changes tested?
- Before the fix, `go test ./parquet/variant -run
TestMetadataOffsetTableCrossesByteBoundary -count=1` reproduces the
panic at `width_2/keys_126`.
- After the fix, `go test ./parquet/variant ./arrow/extensions` passes,
including all 13 new boundary cases.
### Are there any user-facing changes?
Valid VARIANT dictionaries spanning byte 255 decode successfully instead
of panicking. There are no public API or format changes.
Fixes #1291.
---
parquet/variant/variant.go | 10 +++---
.../variant_metadata_offset_boundary_test.go | 41 ++++++++++++++++++++++
2 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/parquet/variant/variant.go b/parquet/variant/variant.go
index a4557004..35f72c73 100644
--- a/parquet/variant/variant.go
+++ b/parquet/variant/variant.go
@@ -200,16 +200,18 @@ func (m *Metadata) loadDictionary(offsetSz uint8) error {
ErrInvalidMetadata, valuesStart, len(m.data))
}
- offsetPos := hdrSizeBytes + offsetSz
- if first := readLEU32(m.data[offsetPos : offsetPos+offsetSz]); first !=
0 {
+ // The table position can exceed 255 even when offsets use only two
bytes.
+ offsetWidth := int(offsetSz)
+ offsetPos := hdrSizeBytes + offsetWidth
+ if first := readLEU32(m.data[offsetPos : offsetPos+offsetWidth]); first
!= 0 {
return fmt.Errorf("%w: first offset must be zero: %d",
ErrInvalidMetadata, first)
}
m.keys = make([][]byte, dictSize)
offsetStart := uint32(0)
for i := range dictSize {
- offsetPos += offsetSz
- end := readLEU32(m.data[offsetPos : offsetPos+offsetSz])
+ offsetPos += offsetWidth
+ end := readLEU32(m.data[offsetPos : offsetPos+offsetWidth])
if end < offsetStart {
return fmt.Errorf("%w: offsets are not monotonic: %d <
%d",
ErrInvalidMetadata, end, offsetStart)
diff --git a/parquet/variant/variant_metadata_offset_boundary_test.go
b/parquet/variant/variant_metadata_offset_boundary_test.go
index ae22fc1c..555b3f4e 100644
--- a/parquet/variant/variant_metadata_offset_boundary_test.go
+++ b/parquet/variant/variant_metadata_offset_boundary_test.go
@@ -17,6 +17,7 @@
package variant_test
import (
+ "fmt"
"testing"
"github.com/apache/arrow-go/v18/parquet/variant"
@@ -37,3 +38,43 @@ func TestMetadataOffsetTableMayEndAtInputBoundary(t
*testing.T) {
require.NoError(t, err)
require.Empty(t, key)
}
+
+// Multi-byte offset tables can cross byte 255 even for small dictionaries.
+func TestMetadataOffsetTableCrossesByteBoundary(t *testing.T) {
+ for _, tc := range []struct{ width, keys int }{
+ {2, 125}, {2, 126}, {2, 127}, {2, 128}, {2, 256},
+ {3, 82}, {3, 83}, {3, 84}, {3, 256},
+ {4, 61}, {4, 62}, {4, 63}, {4, 256},
+ } {
+ t.Run(fmt.Sprintf("width_%d/keys_%d", tc.width, tc.keys),
func(t *testing.T) {
+ // Use explicitly encoded, valid metadata to exercise
each offset width
+ // independently of the builder's choice of the
smallest representation.
+ encoded := []byte{byte(1 | (tc.width-1)<<6)}
+ appendOffset := func(value int) {
+ for i := 0; i < tc.width; i++ {
+ encoded = append(encoded,
byte(value>>(8*i)))
+ }
+ }
+ appendOffset(tc.keys)
+ keys := make([]string, tc.keys)
+ offset := 0
+ for i := range keys {
+ keys[i] = fmt.Sprintf("key_%04d", i)
+ appendOffset(offset)
+ offset += len(keys[i])
+ }
+ appendOffset(offset)
+ for _, key := range keys {
+ encoded = append(encoded, key...)
+ }
+ metadata, err := variant.NewMetadata(encoded)
+ require.NoError(t, err)
+ require.EqualValues(t, tc.keys,
metadata.DictionarySize())
+ for i, want := range keys {
+ got, err := metadata.KeyAt(uint32(i))
+ require.NoError(t, err)
+ require.Equal(t, want, got)
+ }
+ })
+ }
+}
