Skip to content

Commit

Permalink
fix: bounds check
Browse files Browse the repository at this point in the history
  • Loading branch information
steveluscher committed Apr 1, 2024
1 parent 5b21c65 commit 77d9352
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 24 deletions.
17 changes: 9 additions & 8 deletions packages/library-legacy/src/message/legacy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import {TransactionInstruction} from '../transaction';
import {CompiledKeys} from './compiled-keys';
import {MessageAccountKeys} from './account-keys';
import {guardedShift, guardedSplice} from '../utils/guarded-array-utils';

/**
* An instruction to execute by a program
Expand Down Expand Up @@ -268,7 +269,7 @@ export class Message {
// Slice up wire data
let byteArray = [...buffer];

const numRequiredSignatures = byteArray.shift()!;
const numRequiredSignatures = guardedShift(byteArray);
if (
numRequiredSignatures !==
(numRequiredSignatures & VERSION_PREFIX_MASK)
Expand All @@ -278,26 +279,26 @@ export class Message {
);
}

const numReadonlySignedAccounts = byteArray.shift()!;
const numReadonlyUnsignedAccounts = byteArray.shift()!;
const numReadonlySignedAccounts = guardedShift(byteArray);
const numReadonlyUnsignedAccounts = guardedShift(byteArray);

const accountCount = shortvec.decodeLength(byteArray);
let accountKeys = [];
for (let i = 0; i < accountCount; i++) {
const account = byteArray.splice(0, PUBLIC_KEY_LENGTH);
const account = guardedSplice(byteArray, 0, PUBLIC_KEY_LENGTH);
accountKeys.push(new PublicKey(Buffer.from(account)));
}

const recentBlockhash = byteArray.splice(0, PUBLIC_KEY_LENGTH);
const recentBlockhash = guardedSplice(byteArray, 0, PUBLIC_KEY_LENGTH);

const instructionCount = shortvec.decodeLength(byteArray);
let instructions: CompiledInstruction[] = [];
for (let i = 0; i < instructionCount; i++) {
const programIdIndex = byteArray.shift()!;
const programIdIndex = guardedShift(byteArray);
const accountCount = shortvec.decodeLength(byteArray);
const accounts = byteArray.splice(0, accountCount);
const accounts = guardedSplice(byteArray, 0, accountCount);
const dataLength = shortvec.decodeLength(byteArray);
const dataSlice = byteArray.splice(0, dataLength);
const dataSlice = guardedSplice(byteArray, 0, dataLength);
const data = bs58.encode(Buffer.from(dataSlice));
instructions.push({
programIdIndex,
Expand Down
41 changes: 29 additions & 12 deletions packages/library-legacy/src/message/v0.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {TransactionInstruction} from '../transaction';
import {AddressLookupTableAccount} from '../programs';
import {CompiledKeys} from './compiled-keys';
import {AccountKeysFromLookups, MessageAccountKeys} from './account-keys';
import {guardedShift, guardedSplice} from '../utils/guarded-array-utils';

/**
* Message constructor arguments
Expand Down Expand Up @@ -426,7 +427,7 @@ export class MessageV0 {
static deserialize(serializedMessage: Uint8Array): MessageV0 {
let byteArray = [...serializedMessage];

const prefix = byteArray.shift() as number;
const prefix = guardedShift(byteArray);
const maskedPrefix = prefix & VERSION_PREFIX_MASK;
assert(
prefix !== maskedPrefix,
Expand All @@ -440,29 +441,35 @@ export class MessageV0 {
);

const header: MessageHeader = {
numRequiredSignatures: byteArray.shift() as number,
numReadonlySignedAccounts: byteArray.shift() as number,
numReadonlyUnsignedAccounts: byteArray.shift() as number,
numRequiredSignatures: guardedShift(byteArray),
numReadonlySignedAccounts: guardedShift(byteArray),
numReadonlyUnsignedAccounts: guardedShift(byteArray),
};

const staticAccountKeys = [];
const staticAccountKeysLength = shortvec.decodeLength(byteArray);
for (let i = 0; i < staticAccountKeysLength; i++) {
staticAccountKeys.push(
new PublicKey(byteArray.splice(0, PUBLIC_KEY_LENGTH)),
new PublicKey(guardedSplice(byteArray, 0, PUBLIC_KEY_LENGTH)),
);
}

const recentBlockhash = bs58.encode(byteArray.splice(0, PUBLIC_KEY_LENGTH));
const recentBlockhash = bs58.encode(
guardedSplice(byteArray, 0, PUBLIC_KEY_LENGTH),
);

const instructionCount = shortvec.decodeLength(byteArray);
const compiledInstructions: MessageCompiledInstruction[] = [];
for (let i = 0; i < instructionCount; i++) {
const programIdIndex = byteArray.shift() as number;
const programIdIndex = guardedShift(byteArray);
const accountKeyIndexesLength = shortvec.decodeLength(byteArray);
const accountKeyIndexes = byteArray.splice(0, accountKeyIndexesLength);
const accountKeyIndexes = guardedSplice(
byteArray,
0,
accountKeyIndexesLength,
);
const dataLength = shortvec.decodeLength(byteArray);
const data = new Uint8Array(byteArray.splice(0, dataLength));
const data = new Uint8Array(guardedSplice(byteArray, 0, dataLength));
compiledInstructions.push({
programIdIndex,
accountKeyIndexes,
Expand All @@ -473,11 +480,21 @@ export class MessageV0 {
const addressTableLookupsCount = shortvec.decodeLength(byteArray);
const addressTableLookups: MessageAddressTableLookup[] = [];
for (let i = 0; i < addressTableLookupsCount; i++) {
const accountKey = new PublicKey(byteArray.splice(0, PUBLIC_KEY_LENGTH));
const accountKey = new PublicKey(
guardedSplice(byteArray, 0, PUBLIC_KEY_LENGTH),
);
const writableIndexesLength = shortvec.decodeLength(byteArray);
const writableIndexes = byteArray.splice(0, writableIndexesLength);
const writableIndexes = guardedSplice(
byteArray,
0,
writableIndexesLength,
);
const readonlyIndexesLength = shortvec.decodeLength(byteArray);
const readonlyIndexes = byteArray.splice(0, readonlyIndexesLength);
const readonlyIndexes = guardedSplice(
byteArray,
0,
readonlyIndexesLength,
);
addressTableLookups.push({
accountKey,
writableIndexes,
Expand Down
3 changes: 2 additions & 1 deletion packages/library-legacy/src/transaction/legacy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {Signer} from '../keypair';
import type {Blockhash} from '../blockhash';
import type {CompiledInstruction} from '../message';
import {sign, verify} from '../utils/ed25519';
import {guardedSplice} from '../utils/guarded-array-utils';

/** @internal */
type MessageSignednessErrors = {
Expand Down Expand Up @@ -904,7 +905,7 @@ export class Transaction {
const signatureCount = shortvec.decodeLength(byteArray);
let signatures = [];
for (let i = 0; i < signatureCount; i++) {
const signature = byteArray.splice(0, SIGNATURE_LENGTH_IN_BYTES);
const signature = guardedSplice(byteArray, 0, SIGNATURE_LENGTH_IN_BYTES);
signatures.push(bs58.encode(Buffer.from(signature)));
}

Expand Down
3 changes: 2 additions & 1 deletion packages/library-legacy/src/transaction/versioned.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as shortvec from '../utils/shortvec-encoding';
import * as Layout from '../layout';
import {sign} from '../utils/ed25519';
import {PublicKey} from '../publickey';
import {guardedSplice} from '../utils/guarded-array-utils';

export type TransactionVersion = 'legacy' | 0;

Expand Down Expand Up @@ -82,7 +83,7 @@ export class VersionedTransaction {
const signaturesLength = shortvec.decodeLength(byteArray);
for (let i = 0; i < signaturesLength; i++) {
signatures.push(
new Uint8Array(byteArray.splice(0, SIGNATURE_LENGTH_IN_BYTES)),
new Uint8Array(guardedSplice(byteArray, 0, SIGNATURE_LENGTH_IN_BYTES)),
);
}

Expand Down
34 changes: 34 additions & 0 deletions packages/library-legacy/src/utils/guarded-array-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
const END_OF_BUFFER_ERROR_MESSAGE = 'Reached end of buffer unexpectedly';

/**
* Delegates to `Array#shift`, but throws if the array is zero-length.
*/
export function guardedShift<T>(byteArray: T[]): T {
if (byteArray.length === 0) {
throw new Error(END_OF_BUFFER_ERROR_MESSAGE);
}
return byteArray.shift() as T;
}

/**
* Delegates to `Array#splice`, but throws if the section being spliced out extends past the end of
* the array.
*/
export function guardedSplice<T>(
byteArray: T[],
...args:
| [start: number, deleteCount?: number]
| [start: number, deleteCount: number, ...items: T[]]
): T[] {
const [start] = args;
if (
args.length === 2 // Implies that `deleteCount` was supplied
? start + (args[1] ?? 0) > byteArray.length
: start >= byteArray.length
) {
throw new Error(END_OF_BUFFER_ERROR_MESSAGE);
}
return byteArray.splice(
...(args as Parameters<typeof Array.prototype.splice>),
);
}
7 changes: 5 additions & 2 deletions packages/library-legacy/src/validator-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import * as Layout from './layout';
import * as shortvec from './utils/shortvec-encoding';
import {PublicKey, PUBLIC_KEY_LENGTH} from './publickey';
import {guardedShift, guardedSplice} from './utils/guarded-array-utils';

export const VALIDATOR_INFO_KEY = new PublicKey(
'Va1idator1nfo111111111111111111111111111111',
Expand Down Expand Up @@ -83,8 +84,10 @@ export class ValidatorInfo {

const configKeys: Array<ConfigKey> = [];
for (let i = 0; i < 2; i++) {
const publicKey = new PublicKey(byteArray.splice(0, PUBLIC_KEY_LENGTH));
const isSigner = byteArray.splice(0, 1)[0] === 1;
const publicKey = new PublicKey(
guardedSplice(byteArray, 0, PUBLIC_KEY_LENGTH),
);
const isSigner = guardedShift(byteArray) === 1;
configKeys.push({publicKey, isSigner});
}

Expand Down
52 changes: 52 additions & 0 deletions packages/library-legacy/test/guarded-array-utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import {expect} from 'chai';
import {spy} from 'sinon';

import {guardedShift, guardedSplice} from '../src/utils/guarded-array-utils';

describe('guardedShift', () => {
it('delegates to Array#shift', () => {
const arr = [1, 2, 3];
const shiftSpy = spy(arr, 'shift');
const result = guardedShift(arr);
expect(shiftSpy).is.calledWithExactly();
expect(result).to.eq(shiftSpy.returnValues[0]);
});
it('throws when the array is zero-length', () => {
const arr: number[] = [];
expect(() => guardedShift(arr)).to.throw();
});
});

describe('guardedSplice', () => {
it('delegates to Array#splice', () => {
const arr = [1, 2, 3];
const spliceSpy = spy(arr, 'splice');
const result = guardedSplice(
arr,
/* start */ 0,
/* deleteCount */ 3,
/* ...items */
100,
101,
102,
);
expect(spliceSpy).is.calledWithExactly(0, 3, 100, 101, 102);
expect(result).to.eq(spliceSpy.returnValues[0]);
});
it('allows zero-length splices', () => {
const arr: number[] = [1, 2, 3];
expect(guardedSplice(arr, 0, 0)).to.be.an.empty('array');
});
it('allows zero-length splices via the `deleteCount` argument being the explicit value `undefined`', () => {
const arr: number[] = [1, 2, 3];
expect(guardedSplice(arr, 0, undefined)).to.be.an.empty('array');
});
it('throws when the `start` would take you past the end of the array', () => {
const arr: number[] = [1, 2, 3];
expect(() => guardedSplice(arr, 3)).to.throw();
});
it('throws when the `deleteCount` and `start` would take you past the end of the array', () => {
const arr: number[] = [1, 2, 3];
expect(() => guardedSplice(arr, 1, 3)).to.throw();
});
});

0 comments on commit 77d9352

Please sign in to comment.