diff --git a/core/vm/memory.go b/core/vm/memory.go index 8fe575b9aa78..e09859996f70 100644 --- a/core/vm/memory.go +++ b/core/vm/memory.go @@ -40,7 +40,9 @@ func NewMemory() *Memory { } func (m *Memory) Store() []byte { - return m.store + // Clamp the capacity so callers cannot append into the unallocated tail of + // the backing array, which Resize may later hand back as zeroed memory. + return m.store[:len(m.store):len(m.store)] } // Free returns the memory to the pool. @@ -48,6 +50,7 @@ func (m *Memory) Free() { // To reduce peak allocation, return only smaller memory instances to the pool. const maxBufferSize = 16 << 10 if cap(m.store) <= maxBufferSize { + clear(m.store) m.store = m.store[:0] m.lastGasCost = 0 memoryPool.Put(m) @@ -80,10 +83,14 @@ func (m *Memory) Set32(offset uint64, val *uint256.Int) { val.PutUint256(m.store[offset:]) } -// Resize resizes the memory to size +// Resize grows the memory to the requested size. func (m *Memory) Resize(size uint64) { - if uint64(m.Len()) < size { - m.store = append(m.store, make([]byte, size-uint64(m.Len()))...) + if uint64(len(m.store)) < size { + if uint64(cap(m.store)) >= size { + m.store = m.store[:size] + } else { + m.store = append(m.store, make([]byte, size-uint64(len(m.store)))...) + } } } @@ -105,8 +112,11 @@ func (m *Memory) GetPtr(offset, size uint64) []byte { return nil } - // memory is always resized before being accessed, no need to check bounds - return m.store[offset : offset+size] + // memory is always resized before being accessed, no need to check bounds. + // The capacity is clamped so callers cannot append into the unallocated + // tail of the backing array, which Resize may later hand back as zeroed + // memory. + return m.store[offset : offset+size : offset+size] } // Len returns the length of the backing slice @@ -116,7 +126,8 @@ func (m *Memory) Len() int { // Data returns the backing slice func (m *Memory) Data() []byte { - return m.store + // Capacity is clamped, see Store. + return m.store[:len(m.store):len(m.store)] } // Copy copies data from the src position slice into the dst position. diff --git a/core/vm/memory_test.go b/core/vm/memory_test.go index 41389b729aa8..575d2af2d07f 100644 --- a/core/vm/memory_test.go +++ b/core/vm/memory_test.go @@ -83,3 +83,45 @@ func TestMemoryCopy(t *testing.T) { } } } + +func BenchmarkResize(b *testing.B) { + memory := NewMemory() + for i := range b.N { + memory.Resize(uint64(i)) + } +} + +// TestMemoryViewCapacityClamped verifies that the slices handed out by GetPtr, +// Data and Store cannot be appended into the unallocated tail of the backing +// array. Resize reslices within capacity, so a stray append by a caller (e.g. a +// custom precompile) would otherwise make newly expanded memory read non-zero, +// breaking the EVM invariant that fresh memory is zeroed. +func TestMemoryViewCapacityClamped(t *testing.T) { + m := NewMemory() + m.Resize(128) // grow the backing array... + m.store = m.store[:32] // ...then shrink the view, leaving spare capacity + if cap(m.store) < 128 { + t.Fatalf("test precondition: want spare capacity, have cap %d", cap(m.store)) + } + + for _, tc := range []struct { + name string + view []byte + }{ + {"GetPtr", m.GetPtr(0, 32)}, + {"GetPtr offset", m.GetPtr(16, 16)}, + {"Data", m.Data()}, + {"Store", m.Store()}, + } { + if have, want := cap(tc.view), len(tc.view); have != want { + t.Errorf("%s: capacity not clamped: have %d, want %d", tc.name, have, want) + } + // Appending must reallocate rather than write into m.store's tail. + _ = append(tc.view, 0xff) + } + + m.Resize(128) + if want := make([]byte, 96); !bytes.Equal(m.store[32:], want) { + t.Errorf("expanded memory not zeroed: %#x", m.store[32:]) + } +}