feat: throwing errors in BufferReader when out of bounds (#7149)

yarn-project/foundation/src/serialize/buffer_reader.test.ts

@@ -173,4 +173,72 @@ describe('buffer reader', () => {
       expect(bufferReader.peekBytes(10)).toEqual(Buffer.from(ARRAY.slice(0, 10)));
     });
   });
+
+  describe('error handling', () => {
+    let smallBuffer: Buffer;
+    let smallBufferReader: BufferReader;
+
+    beforeEach(() => {
+      smallBuffer = Buffer.from([1, 2, 3]); // 3-byte buffer
+      smallBufferReader = new BufferReader(smallBuffer);
+    });
+
+    it('should throw error when reading number beyond buffer length', () => {
+      expect(() => smallBufferReader.readNumber()).toThrow('Attempted to read beyond buffer length');
+    });
+
+    it('should throw error when reading numbers beyond buffer length', () => {
+      expect(() => smallBufferReader.readNumbers(1)).toThrow('Attempted to read beyond buffer length');
+    });
+
+    it('should throw error when reading UInt16 beyond buffer length', () => {
+      smallBufferReader.readBytes(2);
+      expect(() => smallBufferReader.readUInt16()).toThrow('Attempted to read beyond buffer length');
+    });
+
+    it('should throw error when reading UInt8 beyond buffer length', () => {
+      smallBufferReader.readBytes(3); // Read all bytes
+      expect(() => smallBufferReader.readUInt8()).toThrow('Attempted to read beyond buffer length');
+    });
+
+    it('should throw error when reading boolean beyond buffer length', () => {
+      smallBufferReader.readBytes(3); // Read all bytes
+      expect(() => smallBufferReader.readBoolean()).toThrow('Attempted to read beyond buffer length');
+    });
+
+    it('should throw error when reading bytes beyond buffer length', () => {
+      expect(() => smallBufferReader.readBytes(4)).toThrow('Attempted to read beyond buffer length');
+    });
+
+    it('should throw error when reading buffer beyond buffer length', () => {
+      // First, read a number (4 bytes) which is already beyond the buffer length
+      expect(() => smallBufferReader.readBuffer()).toThrow('Attempted to read beyond buffer length');
+    });
+
+    it('should throw error when peeking beyond buffer length', () => {
+      expect(() => smallBufferReader.peekBytes(4)).toThrow('Attempted to read beyond buffer length');
+    });
+
+    it('should throw error when reading vector beyond buffer length', () => {
+      expect(() => smallBufferReader.readVector({ fromBuffer: () => 1 })).toThrow(
+        'Attempted to read beyond buffer length',
+      );
+    });
+
+    it('should throw error when reading array beyond buffer length', () => {
+      expect(() =>
+        smallBufferReader.readArray(4, { fromBuffer: (reader: BufferReader) => reader.readBytes(1) }),
+      ).toThrow('Attempted to read beyond buffer length');
+    });
+
+    it('should throw error when reading string beyond buffer length', () => {
+      expect(() => smallBufferReader.readString()).toThrow('Attempted to read beyond buffer length');
+    });
+
+    it('should throw error when reading map beyond buffer length', () => {
+      expect(() => smallBufferReader.readMap({ fromBuffer: () => 1 })).toThrow(
+        'Attempted to read beyond buffer length',
+      );
+    });
+  });
 });

yarn-project/foundation/src/serialize/buffer_reader.ts

@@ -55,6 +55,7 @@ export class BufferReader {
    * @returns The read 32-bit unsigned integer value.
    */
   public readNumber(): number {
+    this.#rangeCheck(4);
     this.index += 4;
     return this.buffer.readUint32BE(this.index - 4);
   }
@@ -76,6 +77,7 @@ export class BufferReader {
    * @returns The read 16 bit value.
    */
   public readUInt16(): number {
+    this.#rangeCheck(2);
     this.index += 2;
     return this.buffer.readUInt16BE(this.index - 2);
   }
@@ -87,6 +89,7 @@ export class BufferReader {
    * @returns The read 8 bit value.
    */
   public readUInt8(): number {
+    this.#rangeCheck(1);
     this.index += 1;
     return this.buffer.readUInt8(this.index - 1);
   }
@@ -99,6 +102,7 @@ export class BufferReader {
    * @returns A boolean value representing the byte at the current index.
    */
   public readBoolean(): boolean {
+    this.#rangeCheck(1);
     this.index += 1;
     return Boolean(this.buffer.at(this.index - 1));
   }
@@ -112,6 +116,7 @@ export class BufferReader {
    * @returns A new Buffer containing the read bytes.
    */
   public readBytes(n: number): Buffer {
+    this.#rangeCheck(n);
     this.index += n;
     return Buffer.from(this.buffer.subarray(this.index - n, this.index));
   }
@@ -215,6 +220,7 @@ export class BufferReader {
   public readBufferArray(size = -1): Buffer[] {
     const result: Buffer[] = [];
     const end = size >= 0 ? this.index + size : this.buffer.length;
+    this.#rangeCheck(end - this.index);
     while (this.index < end) {
       const item = this.readBuffer();
       result.push(item);
@@ -252,6 +258,7 @@ export class BufferReader {
    * @returns A Buffer with the next n bytes or the remaining bytes if n is not provided or exceeds the buffer length.
    */
   public peekBytes(n?: number): Buffer {
+    this.#rangeCheck(n || 0);
     return this.buffer.subarray(this.index, n ? this.index + n : undefined);
   }
 
@@ -276,6 +283,7 @@ export class BufferReader {
    */
   public readBuffer(): Buffer {
     const size = this.readNumber();
+    this.#rangeCheck(size);
     return this.readBytes(size);
   }
 
@@ -311,6 +319,14 @@ export class BufferReader {
   public getLength(): number {
     return this.buffer.length;
   }
+
+  #rangeCheck(numBytes: number) {
+    if (this.index + numBytes > this.buffer.length) {
+      throw new Error(
+        `Attempted to read beyond buffer length. Start index: ${this.index}, Num bytes to read: ${numBytes}, Buffer length: ${this.buffer.length}`,
+      );
+    }
+  }
 }
 
 /**

yarn-project/merkle-tree/src/snapshots/indexed_tree_snapshot.test.ts

@@ -44,7 +44,7 @@ describe('IndexedTreeSnapshotBuilder', () => {
 
   describe('getSnapshot', () => {
     it('returns historical leaf data', async () => {
-      await tree.appendLeaves([Buffer.from('a'), Buffer.from('b'), Buffer.from('c')]);
+      await tree.appendLeaves([Fr.random().toBuffer(), Fr.random().toBuffer(), Fr.random().toBuffer()]);
       await tree.commit();
       const expectedLeavesAtBlock1 = await Promise.all([
         tree.getLatestLeafPreimageCopy(0n, false),
@@ -59,7 +59,7 @@ describe('IndexedTreeSnapshotBuilder', () => {
 
       await snapshotBuilder.snapshot(1);
 
-      await tree.appendLeaves([Buffer.from('d'), Buffer.from('e'), Buffer.from('f')]);
+      await tree.appendLeaves([Fr.random().toBuffer(), Fr.random().toBuffer(), Fr.random().toBuffer()]);
       await tree.commit();
       const expectedLeavesAtBlock2 = [
         tree.getLatestLeafPreimageCopy(0n, false),
@@ -98,12 +98,12 @@ describe('IndexedTreeSnapshotBuilder', () => {
 
   describe('findIndexOfPreviousValue', () => {
     it('returns the index of the leaf with the closest value to the given value', async () => {
-      await tree.appendLeaves([Buffer.from('a'), Buffer.from('f'), Buffer.from('d')]);
+      await tree.appendLeaves([Fr.random().toBuffer(), Fr.random().toBuffer(), Fr.random().toBuffer()]);
       await tree.commit();
       const snapshot = await snapshotBuilder.snapshot(1);
       const historicalPrevValue = tree.findIndexOfPreviousKey(2n, false);
 
-      await tree.appendLeaves([Buffer.from('c'), Buffer.from('b'), Buffer.from('e')]);
+      await tree.appendLeaves([Fr.random().toBuffer(), Fr.random().toBuffer(), Fr.random().toBuffer()]);
       await tree.commit();
 
       expect(snapshot.findIndexOfPreviousKey(2n)).toEqual(historicalPrevValue);

yarn-project/prover-client/src/orchestrator/orchestrator.ts

@@ -880,5 +880,9 @@ function extractAggregationObject(proof: Proof, numPublicInputs: number): Fr[] {
     Fr.SIZE_IN_BYTES * (numPublicInputs - AGGREGATION_OBJECT_LENGTH),
     Fr.SIZE_IN_BYTES * numPublicInputs,
   );
+  // TODO(#7159): Remove the following workaround
+  if (buffer.length === 0) {
+    return Array.from({ length: AGGREGATION_OBJECT_LENGTH }, () => Fr.ZERO);
+  }
   return BufferReader.asReader(buffer).readArray(AGGREGATION_OBJECT_LENGTH, Fr);
 }