From d474ac55757a14f604fcec8cedb049318366dd16 Mon Sep 17 00:00:00 2001 From: Brian Shih Date: Sat, 20 Apr 2024 11:03:57 -0400 Subject: [PATCH 1/5] MaxSliceAllocSize --- codec_array.go | 6 ++++++ config.go | 13 +++++++++++++ decoder_array_test.go | 15 +++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/codec_array.go b/codec_array.go index 3dd82fe..e6fdef6 100644 --- a/codec_array.go +++ b/codec_array.go @@ -59,6 +59,12 @@ func (d *arrayDecoder) Decode(ptr unsafe.Pointer, r *Reader) { r.ReportError("decode array", "size exceeded max allocation size") return } + + if max := r.cfg.getMaxSliceAllocSize(); max > 0 && size > max { + 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..9dce77a 100644 --- a/config.go +++ b/config.go @@ -9,6 +9,7 @@ import ( ) const defaultMaxByteSliceSize = 1_048_576 // 1 MiB +const defaultMaxSliceAllocSize = -1 // DefaultConfig is the default API. var DefaultConfig = Config{}.Freeze() @@ -49,6 +50,10 @@ 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, disabled by default. + // If this size is exceeded, the decoder returns an error. This can be disabled by setting a negative number. + MaxSliceAllocSize int } // Freeze makes the configuration immutable. @@ -266,3 +271,11 @@ func (c *frozenConfig) getMaxByteSliceSize() int { } return size } + +func (c *frozenConfig) getMaxSliceAllocSize() int { + size := c.config.MaxSliceAllocSize + if size == 0 { + return defaultMaxSliceAllocSize + } + return size +} 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) +} From a505765f8c115b10dcd1643f723db93218c6cc40 Mon Sep 17 00:00:00 2001 From: Brian Shih Date: Sat, 20 Apr 2024 11:10:12 -0400 Subject: [PATCH 2/5] Lint --- config.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/config.go b/config.go index 9dce77a..fde9c0a 100644 --- a/config.go +++ b/config.go @@ -8,8 +8,10 @@ import ( "github.com/modern-go/reflect2" ) -const defaultMaxByteSliceSize = 1_048_576 // 1 MiB -const defaultMaxSliceAllocSize = -1 +const ( + defaultMaxByteSliceSize = 1_048_576 // 1 MiB + defaultMaxSliceAllocSize = -1 +) // DefaultConfig is the default API. var DefaultConfig = Config{}.Freeze() From 180a476a8d7acfc9a4e2d1c5536b81146ec0846f Mon Sep 17 00:00:00 2001 From: Brian Shih Date: Sat, 20 Apr 2024 12:32:14 -0400 Subject: [PATCH 3/5] combine allocation check logic --- codec_array.go | 10 +--------- config.go | 13 +++++++++---- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/codec_array.go b/codec_array.go index e6fdef6..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,12 +51,8 @@ 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") - return - } - if max := r.cfg.getMaxSliceAllocSize(); max > 0 && size > max { + if size > r.cfg.getMaxSliceAllocSize() { r.ReportError("decode array", "size is greater than `Config.MaxSliceAllocSize`") return } diff --git a/config.go b/config.go index fde9c0a..b644c1c 100644 --- a/config.go +++ b/config.go @@ -53,8 +53,9 @@ type Config struct { // 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, disabled by default. - // If this size is exceeded, the decoder returns an error. This can be disabled by setting a negative number. + // 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 } @@ -274,10 +275,14 @@ func (c *frozenConfig) getMaxByteSliceSize() int { return size } +// 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 = int(1 << 48) + func (c *frozenConfig) getMaxSliceAllocSize() int { size := c.config.MaxSliceAllocSize - if size == 0 { - return defaultMaxSliceAllocSize + if size > maxAllocSize || size <= 0 { + return maxAllocSize } return size } From 80c83c1a422cccbc5a0e35317a958e7a3622f347 Mon Sep 17 00:00:00 2001 From: Brian Shih Date: Sat, 20 Apr 2024 12:34:03 -0400 Subject: [PATCH 4/5] move constant --- config.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/config.go b/config.go index b644c1c..9ff873c 100644 --- a/config.go +++ b/config.go @@ -9,8 +9,11 @@ import ( ) const ( - defaultMaxByteSliceSize = 1_048_576 // 1 MiB - defaultMaxSliceAllocSize = -1 + 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) + defaultMaxSliceAllocSize = maxAllocSize ) // DefaultConfig is the default API. @@ -275,10 +278,6 @@ func (c *frozenConfig) getMaxByteSliceSize() int { return size } -// 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 = int(1 << 48) - func (c *frozenConfig) getMaxSliceAllocSize() int { size := c.config.MaxSliceAllocSize if size > maxAllocSize || size <= 0 { From 0825c2a14b47efa0a74c4d48de2c668836c026a8 Mon Sep 17 00:00:00 2001 From: Brian Shih Date: Sat, 20 Apr 2024 12:39:58 -0400 Subject: [PATCH 5/5] Remove defaultMax variable --- config.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config.go b/config.go index 9ff873c..1fec0cc 100644 --- a/config.go +++ b/config.go @@ -12,8 +12,7 @@ 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) - defaultMaxSliceAllocSize = maxAllocSize + maxAllocSize = int(1 << 48) ) // DefaultConfig is the default API.