chore(avm): Check that slice read/write are not out of memory range

barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp

@@ -12,6 +12,7 @@ enum class AvmError : uint32_t {
     INVALID_TAG_VALUE,
     CHECK_TAG_ERROR,
     ADDR_RES_TAG_ERROR,
+    MEM_SLICE_OUT_OF_RANGE,
     REL_ADDR_OUT_OF_RANGE,
     DIV_ZERO,
     PARSING_ERROR,

barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp

@@ -400,11 +400,11 @@ class AvmTraceBuilder {
     // TODO: remove these once everything is constrained.
     AvmMemoryTag unconstrained_get_memory_tag(AddressWithMode addr);
     bool check_tag(AvmMemoryTag tag, AddressWithMode addr);
-    bool check_tag_range(AvmMemoryTag tag, AddressWithMode start_offset, uint32_t size);
+    bool check_tag_range(AvmMemoryTag tag, uint32_t start_offset, uint32_t size);
     FF unconstrained_read_from_memory(AddressWithMode addr);
-    template <typename T> void read_slice_from_memory(AddressWithMode addr, size_t slice_len, std::vector<T>& slice);
+    template <typename T> AvmError read_slice_from_memory(uint32_t addr, uint32_t slice_len, std::vector<T>& slice);
     void write_to_memory(AddressWithMode addr, FF val, AvmMemoryTag w_tag, bool fix_pc = true);
-    template <typename T> void write_slice_to_memory(AddressWithMode addr, AvmMemoryTag w_tag, const T& slice);
+    template <typename T> AvmError write_slice_to_memory(uint32_t addr, AvmMemoryTag w_tag, const T& slice);
 };
 
 } // namespace bb::avm_trace

yarn-project/simulator/src/avm/avm_memory_types.ts

@@ -15,7 +15,12 @@ import { type FunctionsOf } from '@aztec/foundation/types';
 
 import { strict as assert } from 'assert';
 
-import { InstructionExecutionError, InvalidTagValueError, TagCheckError } from './errors.js';
+import {
+  InstructionExecutionError,
+  InvalidTagValueError,
+  MemorySliceOutOfRangeError,
+  TagCheckError,
+} from './errors.js';
 import { Addressing, AddressingMode } from './opcodes/addressing_mode.js';
 
 /** MemoryValue gathers the common operations for all memory types. */
@@ -271,7 +276,11 @@ export class TaggedMemory implements TaggedMemoryInterface {
 
   public getSlice(offset: number, size: number): MemoryValue[] {
     assert(Number.isInteger(offset) && Number.isInteger(size));
-    assert(offset + size <= TaggedMemory.MAX_MEMORY_SIZE);
+
+    if (offset + size > TaggedMemory.MAX_MEMORY_SIZE) {
+      throw new MemorySliceOutOfRangeError(offset, size);
+    }
+
     const slice = new Array<MemoryValue>(size);
 
     for (let i = 0; i < size; i++) {
@@ -299,7 +308,11 @@ export class TaggedMemory implements TaggedMemoryInterface {
 
   public setSlice(offset: number, slice: MemoryValue[]) {
     assert(Number.isInteger(offset));
-    assert(offset + slice.length <= TaggedMemory.MAX_MEMORY_SIZE);
+
+    if (offset + slice.length > TaggedMemory.MAX_MEMORY_SIZE) {
+      throw new MemorySliceOutOfRangeError(offset, slice.length);
+    }
+
     slice.forEach((element, idx) => {
       this._mem.set(offset + idx, element);
     });

yarn-project/simulator/src/avm/errors.ts

@@ -102,10 +102,21 @@ export class TagCheckError extends AvmExecutionError {
  * Error is thrown when a relative memory address resolved to an offset which
  * is out of range, i.e, greater than maxUint32.
  */
-export class AddressOutOfRangeError extends AvmExecutionError {
+export class RelativeAddressOutOfRangeError extends AvmExecutionError {
   constructor(baseAddr: number, relOffset: number) {
     super(`Address out of range. Base address ${baseAddr}, relative offset ${relOffset}`);
-    this.name = 'AddressOutOfRangeError';
+    this.name = 'RelativeAddressOutOfRangeError';
+  }
+}
+
+/**
+ * Error is thrown when a memory slice contains addresses which are
+ * out of range, i.e, greater than maxUint32.
+ */
+export class MemorySliceOutOfRangeError extends AvmExecutionError {
+  constructor(baseAddr: number, size: number) {
+    super(`Memory slice is out of range. Base address ${baseAddr}, size ${size}`);
+    this.name = 'MemorySliceOutOfRangeError';
   }
 }
 

yarn-project/simulator/src/avm/opcodes/addressing_mode.ts

@@ -1,7 +1,7 @@
 import { strict as assert } from 'assert';
 
 import { TaggedMemory, type TaggedMemoryInterface } from '../avm_memory_types.js';
-import { AddressOutOfRangeError } from '../errors.js';
+import { RelativeAddressOutOfRangeError } from '../errors.js';
 
 export enum AddressingMode {
   DIRECT = 0,
@@ -67,7 +67,7 @@ export class Addressing {
         const baseAddr = Number(mem.get(0).toBigInt());
         resolved[i] += baseAddr;
         if (resolved[i] >= TaggedMemory.MAX_MEMORY_SIZE) {
-          throw new AddressOutOfRangeError(baseAddr, offset);
+          throw new RelativeAddressOutOfRangeError(baseAddr, offset);
         }
       }
       if (mode & AddressingMode.INDIRECT) {

yarn-project/simulator/src/avm/opcodes/ec_add.ts

@@ -84,10 +84,11 @@ export class EcAdd extends Instruction {
       dest = grumpkin.add(p1, p2);
     }
 
-    memory.set(dstOffset, new Field(dest.x));
-    memory.set(dstOffset + 1, new Field(dest.y));
+    // Important to use setSlice() and not set() in the two following statements as
+    // this checks that the offsets lie within memory range.
+    memory.setSlice(dstOffset, [new Field(dest.x), new Field(dest.y)]);
     // Check representation of infinity for grumpkin
-    memory.set(dstOffset + 2, new Uint1(dest.equals(Point.ZERO) ? 1 : 0));
+    memory.setSlice(dstOffset + 2, [new Uint1(dest.equals(Point.ZERO) ? 1 : 0)]);
 
     memory.assert({ reads: 6, writes: 3, addressing });
   }

yarn-project/simulator/src/avm/opcodes/external_calls.ts

@@ -39,10 +39,10 @@ abstract class ExternalCall extends Instruction {
     memory.checkTag(TypeTag.UINT32, argsSizeOffset);
 
     const calldataSize = memory.get(argsSizeOffset).toNumber();
+    const calldata = memory.getSlice(argsOffset, calldataSize).map(f => f.toFr());
     memory.checkTagsRange(TypeTag.FIELD, argsOffset, calldataSize);
 
     const callAddress = memory.getAs<Field>(addrOffset);
-    const calldata = memory.getSlice(argsOffset, calldataSize).map(f => f.toFr());
     // If we are already in a static call, we propagate the environment.
     const callType = context.environment.isStaticCall ? 'STATICCALL' : this.type;
 

yarn-project/simulator/src/avm/opcodes/hashing.test.ts

@@ -1,7 +1,8 @@
 import { keccakf1600, sha256Compression } from '@aztec/foundation/crypto';
 
 import { type AvmContext } from '../avm_context.js';
-import { Field, Uint32, Uint64 } from '../avm_memory_types.js';
+import { Field, TaggedMemory, Uint32, Uint64 } from '../avm_memory_types.js';
+import { MemorySliceOutOfRangeError } from '../errors.js';
 import { initContext, randomMemoryUint32s } from '../fixtures/index.js';
 import { Addressing, AddressingMode } from './addressing_mode.js';
 import { KeccakF1600, Poseidon2, Sha256Compression } from './hashing.js';
@@ -65,6 +66,26 @@ describe('Hashing Opcodes', () => {
         new Field(0x16c877b5b9c04d873218804ccbf65d0eeb12db447f66c9ca26fec380055df7e9n),
       ]);
     });
+
+    it('Should throw an error when input state offsets are out of range.', async () => {
+      const indirect = 0;
+      const inputStateOffset = TaggedMemory.MAX_MEMORY_SIZE - 3;
+      const outputStateOffset = 0;
+
+      await expect(new Poseidon2(indirect, inputStateOffset, outputStateOffset).execute(context)).rejects.toThrow(
+        MemorySliceOutOfRangeError,
+      );
+    });
+
+    it('Should throw an error when output state offsets are out of range.', async () => {
+      const indirect = 0;
+      const inputStateOffset = 0;
+      const outputStateOffset = TaggedMemory.MAX_MEMORY_SIZE - 1;
+
+      await expect(new Poseidon2(indirect, inputStateOffset, outputStateOffset).execute(context)).rejects.toThrow(
+        MemorySliceOutOfRangeError,
+      );
+    });
   });
 
   describe('Keccakf1600', () => {
@@ -94,6 +115,29 @@ describe('Hashing Opcodes', () => {
       const result = context.machineState.memory.getSliceAs<Uint64>(dstOffset, 25);
       expect(result).toEqual(keccakf1600(rawArgs).map(a => new Uint64(a)));
     });
+
+    it('Should throw an error when message input offsets are out of range.', async () => {
+      const indirect = 0;
+      const messageOffset = TaggedMemory.MAX_MEMORY_SIZE - 24;
+      const dstOffset = 200;
+
+      await expect(new KeccakF1600(indirect, dstOffset, messageOffset).execute(context)).rejects.toThrow(
+        MemorySliceOutOfRangeError,
+      );
+    });
+
+    it('Should throw an error when destination offsets are out of range.', async () => {
+      const rawArgs = [...Array(25)].map(_ => 0n);
+      const args = rawArgs.map(a => new Uint64(a));
+      const indirect = 0;
+      const messageOffset = 0;
+      const dstOffset = TaggedMemory.MAX_MEMORY_SIZE - 24;
+      context.machineState.memory.setSlice(messageOffset, args);
+
+      await expect(new KeccakF1600(indirect, dstOffset, messageOffset).execute(context)).rejects.toThrow(
+        MemorySliceOutOfRangeError,
+      );
+    });
   });
 
   describe('Sha256Compression', () => {
@@ -179,5 +223,44 @@ describe('Hashing Opcodes', () => {
       const expectedOutput = sha256Compression(stateArray, inputsArray);
       expect(outputArray).toEqual(expectedOutput);
     });
+
+    it('Should throw an error when input offsets are out of range.', async () => {
+      const indirect = 0;
+      const stateOffset = 0;
+      const inputsOffset = TaggedMemory.MAX_MEMORY_SIZE - 15;
+      const outputOffset = 300;
+
+      await expect(
+        new Sha256Compression(indirect, outputOffset, stateOffset, inputsOffset).execute(context),
+      ).rejects.toThrow(MemorySliceOutOfRangeError);
+    });
+
+    it('Should throw an error when state offsets are out of range.', async () => {
+      const inputs = randomMemoryUint32s(16);
+      const indirect = 0;
+      const stateOffset = TaggedMemory.MAX_MEMORY_SIZE - 7;
+      const inputsOffset = 200;
+      const outputOffset = 300;
+      context.machineState.memory.setSlice(inputsOffset, inputs);
+
+      await expect(
+        new Sha256Compression(indirect, outputOffset, stateOffset, inputsOffset).execute(context),
+      ).rejects.toThrow(MemorySliceOutOfRangeError);
+    });
+
+    it('Should throw an error when output offsets are out of range.', async () => {
+      const state = randomMemoryUint32s(8);
+      const inputs = randomMemoryUint32s(16);
+      const indirect = 0;
+      const stateOffset = 0;
+      const inputsOffset = 200;
+      const outputOffset = TaggedMemory.MAX_MEMORY_SIZE - 7;
+      context.machineState.memory.setSlice(stateOffset, state);
+      context.machineState.memory.setSlice(inputsOffset, inputs);
+
+      await expect(
+        new Sha256Compression(indirect, outputOffset, stateOffset, inputsOffset).execute(context),
+      ).rejects.toThrow(MemorySliceOutOfRangeError);
+    });
   });
 });

yarn-project/simulator/src/avm/opcodes/hashing.ts

@@ -30,9 +30,10 @@ export class Poseidon2 extends Instruction {
     const operands = [this.inputStateOffset, this.outputStateOffset];
     const addressing = Addressing.fromWire(this.indirect, operands.length);
     const [inputOffset, outputOffset] = addressing.resolve(operands, memory);
-    memory.checkTagsRange(TypeTag.FIELD, inputOffset, Poseidon2.stateSize);
 
     const inputState = memory.getSlice(inputOffset, Poseidon2.stateSize);
+    memory.checkTagsRange(TypeTag.FIELD, inputOffset, Poseidon2.stateSize);
+
     const outputState = poseidon2Permutation(inputState);
     memory.setSlice(
       outputOffset,
@@ -68,9 +69,9 @@ export class KeccakF1600 extends Instruction {
     const [dstOffset, inputOffset] = addressing.resolve(operands, memory);
     context.machineState.consumeGas(this.gasCost());
 
+    const stateData = memory.getSlice(inputOffset, inputSize).map(word => word.toBigInt());
     memory.checkTagsRange(TypeTag.UINT64, inputOffset, inputSize);
 
-    const stateData = memory.getSlice(inputOffset, inputSize).map(word => word.toBigInt());
     const updatedState = keccakf1600(stateData);
 
     const res = updatedState.map(word => new Uint64(word));
@@ -113,11 +114,12 @@ export class Sha256Compression extends Instruction {
 
     // Note: size of output is same as size of state
     context.machineState.consumeGas(this.gasCost());
+    const inputs = Uint32Array.from(memory.getSlice(inputsOffset, INPUTS_SIZE).map(word => word.toNumber()));
+    const state = Uint32Array.from(memory.getSlice(stateOffset, STATE_SIZE).map(word => word.toNumber()));
+
     memory.checkTagsRange(TypeTag.UINT32, inputsOffset, INPUTS_SIZE);
     memory.checkTagsRange(TypeTag.UINT32, stateOffset, STATE_SIZE);
 
-    const state = Uint32Array.from(memory.getSlice(stateOffset, STATE_SIZE).map(word => word.toNumber()));
-    const inputs = Uint32Array.from(memory.getSlice(inputsOffset, INPUTS_SIZE).map(word => word.toNumber()));
     const output = sha256Compression(state, inputs);
 
     // Conversion required from Uint32Array to Uint32[] (can't map directly, need `...`)

yarn-project/simulator/src/avm/opcodes/memory.test.ts

@@ -1,7 +1,8 @@
 import { Fr } from '@aztec/foundation/fields';
 
 import { type AvmContext } from '../avm_context.js';
-import { Field, TypeTag, Uint8, Uint16, Uint32, Uint64, Uint128 } from '../avm_memory_types.js';
+import { Field, TaggedMemory, TypeTag, Uint8, Uint16, Uint32, Uint64, Uint128 } from '../avm_memory_types.js';
+import { MemorySliceOutOfRangeError } from '../errors.js';
 import { initContext, initExecutionEnvironment } from '../fixtures/index.js';
 import { Opcode } from '../serialization/instruction_serialization.js';
 import { Addressing, AddressingMode } from './addressing_mode.js';
@@ -421,7 +422,21 @@ describe('Memory instructions', () => {
       expect(actual).toEqual([new Field(2), new Field(3)]);
     });
 
-    // TODO: check bad cases (i.e., out of bounds)
+    it('Should return error when memory slice calldatacopy target is out-of-range', async () => {
+      const calldata = [new Fr(1n), new Fr(2n), new Fr(3n)];
+      context = initContext({ env: initExecutionEnvironment({ calldata }) });
+      context.machineState.memory.set(0, new Uint32(0)); // cdoffset
+      context.machineState.memory.set(1, new Uint32(3)); // size
+
+      await expect(
+        new CalldataCopy(
+          /*indirect=*/ 0,
+          /*cdOffset=*/ 0,
+          /*copySize=*/ 1,
+          /*dstOffset=*/ TaggedMemory.MAX_MEMORY_SIZE - 2,
+        ).execute(context),
+      ).rejects.toThrow(MemorySliceOutOfRangeError);
+    });
   });
 
   describe('RETURNDATASIZE', () => {
@@ -505,6 +520,20 @@ describe('Memory instructions', () => {
       expect(actual).toEqual([new Field(2), new Field(3)]);
     });
 
-    // TODO: check bad cases (i.e., out of bounds)
+    it('Should return error when memory slice target is out-of-range', async () => {
+      context = initContext();
+      context.machineState.nestedReturndata = [new Fr(1n), new Fr(2n), new Fr(3n)];
+      context.machineState.memory.set(0, new Uint32(1)); // rdoffset
+      context.machineState.memory.set(1, new Uint32(2)); // size
+
+      await expect(
+        new ReturndataCopy(
+          /*indirect=*/ 0,
+          /*rdOffset=*/ 0,
+          /*copySize=*/ 1,
+          /*dstOffset=*/ TaggedMemory.MAX_MEMORY_SIZE - 1,
+        ).execute(context),
+      ).rejects.toThrow(MemorySliceOutOfRangeError);
+    });
   });
 });

yarn-project/simulator/src/avm/opcodes/misc.ts

@@ -39,14 +39,15 @@ export class DebugLog extends Instruction {
 
     memory.checkTag(TypeTag.UINT32, fieldsSizeOffset);
     const fieldsSize = memory.get(fieldsSizeOffset).toNumber();
+
+    const rawMessage = memory.getSlice(messageOffset, this.messageSize);
+    const fields = memory.getSlice(fieldsOffset, fieldsSize);
+
     memory.checkTagsRange(TypeTag.UINT8, messageOffset, this.messageSize);
     memory.checkTagsRange(TypeTag.FIELD, fieldsOffset, fieldsSize);
 
     context.machineState.consumeGas(this.gasCost(this.messageSize + fieldsSize));
 
-    const rawMessage = memory.getSlice(messageOffset, this.messageSize);
-    const fields = memory.getSlice(fieldsOffset, fieldsSize);
-
     // Interpret str<N> = [u8; N] to string.
     const messageAsStr = rawMessage.map(field => String.fromCharCode(field.toNumber())).join('');
     const formattedStr = applyStringFormatting(

yarn-project/simulator/src/avm/opcodes/multi_scalar_mul.ts

@@ -46,6 +46,12 @@ export class MultiScalarMul extends Instruction {
     if (pointsReadLength % 3 !== 0) {
       throw new InstructionExecutionError(`Points vector offset should be a multiple of 3, was ${pointsReadLength}`);
     }
+
+    // Get the unrolled (x, y, inf) representing the points
+    // Important to perform this before tag validation, as getSlice() first checks
+    // that the slice is not out of memory range. This needs to be aligned with circuit.
+    const pointsVector = memory.getSlice(pointsOffset, pointsReadLength);
+
     // Divide by 3 since each point is represented as a triplet to get the number of points
     const numPoints = pointsReadLength / 3;
     // The tag for each triplet will be (Field, Field, Uint8)
@@ -56,8 +62,6 @@ export class MultiScalarMul extends Instruction {
       // Check Uint1 (inf flag)
       memory.checkTag(TypeTag.UINT1, offset + 2);
     }
-    // Get the unrolled (x, y, inf) representing the points
-    const pointsVector = memory.getSlice(pointsOffset, pointsReadLength);
 
     // The size of the scalars vector is twice the NUMBER of points because of the scalar limb decomposition
     const scalarReadLength = numPoints * 2;
@@ -106,10 +110,11 @@ export class MultiScalarMul extends Instruction {
       }
     }, grumpkin.mul(firstBaseScalarPair[0], firstBaseScalarPair[1]));
 
-    memory.set(outputOffset, new Field(outputPoint.x));
-    memory.set(outputOffset + 1, new Field(outputPoint.y));
+    // Important to use setSlice() and not set() in the two following statements as
+    // this checks that the offsets lie within memory range.
+    memory.setSlice(outputOffset, [new Field(outputPoint.x), new Field(outputPoint.y)]);
     // Check representation of infinity for grumpkin
-    memory.set(outputOffset + 2, new Uint1(outputPoint.equals(Point.ZERO) ? 1 : 0));
+    memory.setSlice(outputOffset + 2, [new Uint1(outputPoint.equals(Point.ZERO) ? 1 : 0)]);
 
     memory.assert({
       reads: 1 + pointsReadLength + scalarReadLength /* points and scalars */,