Skip to content

Commit

Permalink
feat: throwing errors in buffer reader when out of bounds
Browse files Browse the repository at this point in the history
  • Loading branch information
benesjan committed Jun 21, 2024
1 parent 9c52d47 commit 983b48f
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 0 deletions.
64 changes: 64 additions & 0 deletions yarn-project/foundation/src/serialize/buffer_reader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,68 @@ 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 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',
);
});
});
});
14 changes: 14 additions & 0 deletions yarn-project/foundation/src/serialize/buffer_reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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));
}
Expand All @@ -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));
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand All @@ -276,6 +283,7 @@ export class BufferReader {
*/
public readBuffer(): Buffer {
const size = this.readNumber();
this.#rangeCheck(size);
return this.readBytes(size);
}

Expand Down Expand Up @@ -311,6 +319,12 @@ 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');
}
}
}

/**
Expand Down

0 comments on commit 983b48f

Please sign in to comment.