Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/transaction-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Trigger the first-time-interaction warning correctly for `safeTransferFrom` token transfers by including `TransactionType.tokenMethodSafeTransferFrom` in the effective-recipient decoding logic ([#8723](https://github.com/MetaMask/core/pull/8723))

### Fixed

- Reject transactions with a missing `to` and empty `data` to prevent accidental empty contract deployments ([#8744](https://github.com/MetaMask/core/pull/8744))

## [65.2.0]

### Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ describe('determineTransactionType', () => {
});
});

it('returns a contract deployment type when "to" is falsy and there is data', async () => {
it('returns a contract deployment type when "to" is falsy and there is real bytecode', async () => {
const result = await determineTransactionType(
{
...txParams,
Expand All @@ -180,6 +180,34 @@ describe('determineTransactionType', () => {
});
});

it('does NOT classify as deployContract when "to" is falsy but data is "0x" (empty bytecode)', async () => {
jest.mocked(rpcRequest).mockResolvedValue('0x');

const result = await determineTransactionType(
{
...txParams,
to: '',
data: '0x',
},
{ messenger: messengerMock, networkClientId: networkClientIdMock },
);
expect(result.type).not.toBe(TransactionType.deployContract);
});

it('does NOT classify as deployContract when "to" is undefined but data is "0x"', async () => {
jest.mocked(rpcRequest).mockResolvedValue('0x');

const result = await determineTransactionType(
{
...txParams,
to: undefined,
data: '0x',
},
{ messenger: messengerMock, networkClientId: networkClientIdMock },
);
expect(result.type).not.toBe(TransactionType.deployContract);
});

it('returns a simple send type with a 0x getCodeResponse when there is data, but the "to" address is not a contract address', async () => {
jest.mocked(rpcRequest).mockResolvedValue('0x');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ export async function determineTransactionType(
): Promise<InferTransactionTypeResult> {
const { data, to } = txParams;

if (data && !to) {
const hasRealBytecode = Boolean(data && data !== '0x' && data.length > 2);

if (hasRealBytecode && !to) {
return { type: TransactionType.deployContract, getCodeResponse: undefined };
}

Expand Down
72 changes: 67 additions & 5 deletions packages/transaction-controller/src/utils/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,35 +77,97 @@ describe('validation', () => {
);
});

it('should throw if no data', () => {
it('should throw if to is missing and no real bytecode', () => {
const expectedError = rpcErrors.invalidParams(
'Invalid "to" address: must be specified for transactions without contract deployment bytecode.',
);

expect(() =>
validateTxParams({
from: FROM_MOCK,
to: '0x',
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any),
).toThrow(rpcErrors.invalidParams('Invalid "to" address.'));
).toThrow(expectedError);

expect(() =>
validateTxParams({
from: FROM_MOCK,
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any),
).toThrow(rpcErrors.invalidParams('Invalid "to" address.'));
).toThrow(expectedError);
});

it('should throw if to is empty string and no real bytecode', () => {
expect(() =>
validateTxParams({
from: FROM_MOCK,
to: '' as Hex,
data: '0x',
value: '0x1',
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any),
).toThrow(
rpcErrors.invalidParams(
'Invalid "to" address: must be specified for transactions without contract deployment bytecode.',
),
);
});

it('should delete data', () => {
it('should throw if to is empty string and data is "0x" (would deploy empty contract)', () => {
expect(() =>
validateTxParams({
from: FROM_MOCK,
to: '' as Hex,
data: '0x',
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any),
).toThrow(
rpcErrors.invalidParams(
'Invalid "to" address: must be specified for transactions without contract deployment bytecode.',
),
);
});

it('should throw if to is undefined and data is "0x" (would deploy empty contract)', () => {
expect(() =>
validateTxParams({
from: FROM_MOCK,
data: '0x',
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any),
).toThrow(
rpcErrors.invalidParams(
'Invalid "to" address: must be specified for transactions without contract deployment bytecode.',
),
);
});

it('should remove "to" when missing and data has real bytecode (legitimate deployment)', () => {
const transaction = {
data: 'foo',
data: '0x608060405234',
from: TO_MOCK,
to: '0x',
};
validateTxParams(transaction);
expect(transaction.to).toBeUndefined();
});

it('should remove "to" when empty string and data has real bytecode', () => {
const transaction = {
data: '0x608060405234',
from: TO_MOCK,
to: '' as Hex,
};
validateTxParams(transaction);
expect(transaction.to).toBeUndefined();
});

it('should throw if invalid to address', () => {
expect(() =>
validateTxParams({
Expand Down
27 changes: 21 additions & 6 deletions packages/transaction-controller/src/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,16 +205,31 @@ function validateParamValue(value?: string): void {
*
* @param txParams - The transaction parameters object to validate.
* @throws Throws an error if the recipient address is invalid:
* - If the recipient address is an empty string ('0x') or undefined and the transaction contains data,
* the "to" field is removed from the transaction parameters.
* - If the recipient address is not a valid hexadecimal Ethereum address, an error is thrown.
* - If the recipient address is missing (empty string, '0x', or undefined) and the
* transaction does not contain real bytecode (data must be longer than `0x`),
* an error is thrown. This prevents accidental contract deployments with empty
* `to` and empty `data` from locking funds.
* - If the recipient address is missing and the transaction contains real
* bytecode (data longer than `0x`), the "to" field is removed from the
* transaction parameters (legitimate contract deployment).
* - If the recipient address is not a valid hexadecimal Ethereum address, an
* error is thrown.
*/
function validateParamRecipient(txParams: TransactionParams): void {
if (txParams.to === '0x' || txParams.to === undefined) {
if (txParams.data) {
const isMissingRecipient =
txParams.to === '0x' || txParams.to === '' || txParams.to === undefined;

if (isMissingRecipient) {
const hasRealBytecode = Boolean(
txParams.data && txParams.data !== '0x' && txParams.data.length > 2,
);

if (hasRealBytecode) {
delete txParams.to;
} else {
throw rpcErrors.invalidParams(`Invalid "to" address.`);
throw rpcErrors.invalidParams(
`Invalid "to" address: must be specified for transactions without contract deployment bytecode.`,
);
}
} else if (txParams.to !== undefined && !isValidHexAddress(txParams.to)) {
throw rpcErrors.invalidParams(`Invalid "to" address.`);
Expand Down
Loading