diff --git a/yarn-project/foundation/src/serialize/buffer_reader.test.ts b/yarn-project/foundation/src/serialize/buffer_reader.test.ts index f600942aba9f..0845ff591e29 100644 --- a/yarn-project/foundation/src/serialize/buffer_reader.test.ts +++ b/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', + ); + }); + }); }); diff --git a/yarn-project/foundation/src/serialize/buffer_reader.ts b/yarn-project/foundation/src/serialize/buffer_reader.ts index be1bf669a823..caee2973dfc3 100644 --- a/yarn-project/foundation/src/serialize/buffer_reader.ts +++ b/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}`, + ); + } + } } /** diff --git a/yarn-project/merkle-tree/src/snapshots/indexed_tree_snapshot.test.ts b/yarn-project/merkle-tree/src/snapshots/indexed_tree_snapshot.test.ts index aa374542a9f6..75679d4904a3 100644 --- a/yarn-project/merkle-tree/src/snapshots/indexed_tree_snapshot.test.ts +++ b/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); diff --git a/yarn-project/prover-client/src/orchestrator/orchestrator.ts b/yarn-project/prover-client/src/orchestrator/orchestrator.ts index 99526b8dfd1b..5e9469d4265f 100644 --- a/yarn-project/prover-client/src/orchestrator/orchestrator.ts +++ b/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); }