diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 171c33c0a7d..2e48f47ca0e 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Prevent external transactions to internal accounts if `data` included ([#5418](https://github.com/MetaMask/core/pull/5418)) + ## [48.0.0] ### Changed diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index a1a994e6b1c..3b6f7a7b868 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -1096,6 +1096,7 @@ export class TransactionController extends BaseController< const internalAccounts = this.#getInternalAccounts(); await validateTransactionOrigin({ + data: txParams.data, from: txParams.from, internalAccounts, origin, diff --git a/packages/transaction-controller/src/utils/validation.test.ts b/packages/transaction-controller/src/utils/validation.test.ts index 06593d5e8bd..624de7ee8c9 100644 --- a/packages/transaction-controller/src/utils/validation.test.ts +++ b/packages/transaction-controller/src/utils/validation.test.ts @@ -10,6 +10,7 @@ import { import { TransactionEnvelopeType, TransactionType } from '../types'; import type { TransactionParams } from '../types'; +const DATA_MOCK = '0x12345678'; const FROM_MOCK = '0x1678a085c290ebd122dc42cba69373b5953b831d'; const TO_MOCK = '0xfbb5595c18ca76bab52d66188e4ca50c7d95f77a'; const ORIGIN_MOCK = 'test-origin'; @@ -680,9 +681,10 @@ describe('validation', () => { ); }); - it('throws if external and to is internal account', async () => { + it('throws if external and to is internal account and data', async () => { await expect( validateTransactionOrigin({ + data: DATA_MOCK, from: FROM_MOCK, internalAccounts: [TO_MOCK], origin: ORIGIN_MOCK, @@ -693,11 +695,33 @@ describe('validation', () => { }), ).rejects.toThrow( rpcErrors.invalidParams( - 'External transactions to internal accounts are not supported', + 'External transactions to internal accounts cannot include data', ), ); }); + it.each([ + ['undefined', undefined], + ['empty', ''], + ['empty hex', '0x'], + ])( + 'does not throw if external and to is internal account but data is %s', + async (_title, data) => { + expect( + await validateTransactionOrigin({ + data, + from: FROM_MOCK, + internalAccounts: [TO_MOCK], + origin: ORIGIN_MOCK, + selectedAddress: '0x123', + txParams: { + to: TO_MOCK, + } as TransactionParams, + }), + ).toBeUndefined(); + }, + ); + it('does not throw if external and to is internal account but type is batch', async () => { expect( await validateTransactionOrigin({ @@ -752,9 +776,7 @@ describe('validation', () => { }, }), ).toThrow( - rpcErrors.invalidParams( - 'External transactions to internal accounts are not supported', - ), + rpcErrors.invalidParams('Calls to internal accounts are not supported'), ); }); diff --git a/packages/transaction-controller/src/utils/validation.ts b/packages/transaction-controller/src/utils/validation.ts index a490c1f8be6..829c3acfdf6 100644 --- a/packages/transaction-controller/src/utils/validation.ts +++ b/packages/transaction-controller/src/utils/validation.ts @@ -28,6 +28,7 @@ type GasFieldsToValidate = * Validates whether a transaction initiated by a specific 'from' address is permitted by the origin. * * @param options - Options bag. + * @param options.data - The data included in the transaction. * @param options.from - The address from which the transaction is initiated. * @param options.internalAccounts - The internal accounts added to the wallet. * @param options.origin - The origin or source of the transaction. @@ -38,6 +39,7 @@ type GasFieldsToValidate = * @throws Throws an error if the transaction is not permitted. */ export async function validateTransactionOrigin({ + data, from, internalAccounts, origin, @@ -46,6 +48,7 @@ export async function validateTransactionOrigin({ txParams, type, }: { + data?: string; from: string; internalAccounts?: string[]; origin?: string; @@ -82,15 +85,18 @@ export async function validateTransactionOrigin({ ); } + const hasData = Boolean(data && data !== '0x'); + if ( isExternal && + hasData && internalAccounts?.some( (account) => account.toLowerCase() === to?.toLowerCase(), ) && type !== TransactionType.batch ) { throw rpcErrors.invalidParams( - 'External transactions to internal accounts are not supported', + 'External transactions to internal accounts cannot include data', ); } } @@ -279,7 +285,7 @@ export function validateBatchRequest({ ) ) { throw rpcErrors.invalidParams( - 'External transactions to internal accounts are not supported', + 'Calls to internal accounts are not supported', ); } }