Repository: hamba/avro
Pull request #376 (2024-04-20 15:07:47)

feat: add max slice alloc size config
I introduced the `defaultMaxSliceAllocSize` config for security reasons, similar to `Config.MaxByteSliceSize` introduced in this [PR](https://github.com/brianshih1/avro/commit/b4a402f41cf44b6094b5131286830ba9bb1eb290).

I need a way to prevent the array decoder from consuming too much memory for my application, the `defaultMaxSliceAllocSize` is the limit for how much memory the `arrayDecoder` can allocate.

Let me know if you think I should just reuse `MaxByteSliceSize` instead of introducing a new config.


Patch:
diff --git a/codec_array.go b/codec_array.go
index 3dd82fe..53de040 100644
--- a/codec_array.go
+++ b/codec_array.go
@@ -39,10 +39,6 @@ type arrayDecoder struct {
 	decoder ValDecoder
 }
 
-// Max allocation size for an array due to the limit in number of bits in a heap address:
-// https://github.com/golang/go/blob/7f76c00fc5678fa782708ba8fece63750cb89d03/src/runtime/malloc.go#L183
-var maxAllocSize = uint64(1 << 48)
-
 func (d *arrayDecoder) Decode(ptr unsafe.Pointer, r *Reader) {
 	var size int
 	sliceType := d.typ
@@ -55,10 +51,12 @@ func (d *arrayDecoder) Decode(ptr unsafe.Pointer, r *Reader) {
 
 		start := size
 		size += int(l)
-		if size > int(maxAllocSize) {
-			r.ReportError("decode array", "size exceeded max allocation size")
+
+		if size > r.cfg.getMaxSliceAllocSize() {
+			r.ReportError("decode array", "size is greater than `Config.MaxSliceAllocSize`")
 			return
 		}
+
 		sliceType.UnsafeGrow(ptr, size)
 
 		for i := start; i < size; i++ {
diff --git a/config.go b/config.go
index 647ead4..1fec0cc 100644
--- a/config.go
+++ b/config.go
@@ -8,7 +8,12 @@ import (
 	"github.com/modern-go/reflect2"
 )
 
-const defaultMaxByteSliceSize = 1_048_576 // 1 MiB
+const (
+	defaultMaxByteSliceSize = 1_048_576 // 1 MiB
+	// Max allocation size for an array due to the limit in number of bits in a heap address:
+	// https://github.com/golang/go/blob/7f76c00fc5678fa782708ba8fece63750cb89d03/src/runtime/malloc.go#L183
+	maxAllocSize = int(1 << 48)
+)
 
 // DefaultConfig is the default API.
 var DefaultConfig = Config{}.Freeze()
@@ -49,6 +54,11 @@ type Config struct {
 	// MaxByteSliceSize is the maximum size of `bytes` or `string` types the Reader will create, defaulting to 1MiB.
 	// If this size is exceeded, the Reader returns an error. This can be disabled by setting a negative number.
 	MaxByteSliceSize int
+
+	// MaxSliceAllocSize is the maximum size that the decoder will allocate, set to the max heap
+	// allocation size by default.
+	// If this size is exceeded, the decoder returns an error.
+	MaxSliceAllocSize int
 }
 
 // Freeze makes the configuration immutable.
@@ -266,3 +276,11 @@ func (c *frozenConfig) getMaxByteSliceSize() int {
 	}
 	return size
 }
+
+func (c *frozenConfig) getMaxSliceAllocSize() int {
+	size := c.config.MaxSliceAllocSize
+	if size > maxAllocSize || size <= 0 {
+		return maxAllocSize
+	}
+	return size
+}


Test patch:
diff --git a/decoder_array_test.go b/decoder_array_test.go
index f50ec3b..0e8e520 100644
--- a/decoder_array_test.go
+++ b/decoder_array_test.go
@@ -91,3 +91,18 @@ func TestDecoder_ArrayMaxAllocationError(t *testing.T) {
 
 	assert.Error(t, err)
 }
+
+func TestDecoder_ArrayExceedMaxSliceAllocationConfig(t *testing.T) {
+	avro.DefaultConfig = avro.Config{MaxSliceAllocSize: 5}.Freeze()
+
+	// 10 (long) gets encoded to 0x14
+	data := []byte{0x14}
+	schema := `{"type":"array", "items": { "type": "boolean" }}`
+	dec, err := avro.NewDecoder(schema, bytes.NewReader(data))
+	require.NoError(t, err)
+
+	var got []bool
+	err = dec.Decode(&got)
+
+	assert.Error(t, err)
+}