(arrow-go) branch main updated: fix(parquet/variant): prevent metadata offset cursor overflow (#1290)

[ze...@apache.org]

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 <ma...@gmail.com>
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)
+			}
+		})
+	}
+}