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 2b7e5b09 fix(parquet/variant): validate metadata offset tables (#1063)
2b7e5b09 is described below
commit 2b7e5b09ca46c872f0c7fa10d605ee0ebc72bc7c
Author: Minh Vu <[email protected]>
AuthorDate: Wed Aug 5 19:41:38 2026 +0200
fix(parquet/variant): validate metadata offset tables (#1063)
## 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`
---
parquet/variant/variant.go | 36 +++++++++++---------
.../variant_metadata_offset_boundary_test.go | 39 ++++++++++++++++++++++
parquet/variant/variant_test.go | 20 +++++++++++
3 files changed, 80 insertions(+), 15 deletions(-)
diff --git a/parquet/variant/variant.go b/parquet/variant/variant.go
index 8e31969c..2eec9602 100644
--- a/parquet/variant/variant.go
+++ b/parquet/variant/variant.go
@@ -183,32 +183,38 @@ func (m *Metadata) loadDictionary(offsetSz uint8) error {
}
dictSize := readLEU32(m.data[hdrSizeBytes : hdrSizeBytes+offsetSz])
- m.keys = make([][]byte, dictSize)
-
if dictSize == 0 {
+ m.keys = nil
return nil
}
- // first offset is always 0
- offsetStart, offsetPos := uint32(0), hdrSizeBytes+offsetSz
- valuesStart := hdrSizeBytes + (dictSize+2)*uint32(offsetSz)
- if hdrSizeBytes+int(dictSize+1)*int(offsetSz) > len(m.data) {
- 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)) {
+ return fmt.Errorf("%w: offset table out of range: %d > %d",
+ ErrInvalidMetadata, valuesStart, len(m.data))
+ }
+
+ offsetPos := hdrSizeBytes + offsetSz
+ if first := readLEU32(m.data[offsetPos : offsetPos+offsetSz]); 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])
-
- keySize := end - offsetStart
- valStart := valuesStart + offsetStart
- if valStart+keySize > uint32(len(m.data)) {
+ if end < offsetStart {
+ return fmt.Errorf("%w: offsets are not monotonic: %d <
%d",
+ ErrInvalidMetadata, end, offsetStart)
+ }
+ if valuesStart+uint64(end) > uint64(len(m.data)) {
return fmt.Errorf("%w: string data out of range: %d +
%d > %d",
- ErrInvalidMetadata, valStart, keySize,
len(m.data))
+ ErrInvalidMetadata, valuesStart, end,
len(m.data))
}
- m.keys[i] = m.data[valStart : valStart+keySize]
- offsetStart += keySize
+
+ m.keys[i] = m.data[valuesStart+uint64(offsetStart) :
valuesStart+uint64(end)]
+ offsetStart = end
}
return nil
diff --git a/parquet/variant/variant_metadata_offset_boundary_test.go
b/parquet/variant/variant_metadata_offset_boundary_test.go
new file mode 100644
index 00000000..ae22fc1c
--- /dev/null
+++ b/parquet/variant/variant_metadata_offset_boundary_test.go
@@ -0,0 +1,39 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package variant_test
+
+import (
+ "testing"
+
+ "github.com/apache/arrow-go/v18/parquet/variant"
+ "github.com/stretchr/testify/require"
+)
+
+func TestMetadataOffsetTableMayEndAtInputBoundary(t *testing.T) {
+ metadata, err := variant.NewMetadata([]byte{
+ 0x01, // version 1 with one-byte offsets
+ 0x01, // one dictionary key
+ 0x00, // first offset
+ 0x00, // final offset: the key is empty
+ })
+ require.NoError(t, err)
+ require.EqualValues(t, 1, metadata.DictionarySize())
+
+ key, err := metadata.KeyAt(0)
+ require.NoError(t, err)
+ require.Empty(t, key)
+}
diff --git a/parquet/variant/variant_test.go b/parquet/variant/variant_test.go
index 37bc04c8..e8216013 100644
--- a/parquet/variant/variant_test.go
+++ b/parquet/variant/variant_test.go
@@ -574,6 +574,26 @@ func TestInvalidMetadata(t *testing.T) {
metadata: []byte{0x01, 0xFF, 0x00}, // Claims to have
many keys but doesn't
errMsg: "out of range",
},
+ {
+ name: "truncated offset table",
+ metadata: []byte{0x01, 0x01, 0x00},
+ errMsg: "offset table out of range",
+ },
+ {
+ name: "dictionary size overflows offset table",
+ metadata: []byte{0xC1, 0xFF, 0xFF, 0xFF, 0xFF},
+ errMsg: "offset table out of range",
+ },
+ {
+ name: "nonzero first offset",
+ metadata: []byte{0x01, 0x01, 0x01, 0x01},
+ errMsg: "first offset must be zero",
+ },
+ {
+ name: "decreasing offsets",
+ metadata: []byte{0x01, 0x02, 0x00, 0x02, 0x01, 'a',
'b'},
+ errMsg: "offsets are not monotonic",
+ },
{
name: "string data out of range",
metadata: []byte{0x01, 0x01, 0x00, 0x05},
