diff --git a/app/scripts/lib/tx-verification/tx-verification-middleware.ts b/app/scripts/lib/tx-verification/tx-verification-middleware.ts index 46bbce9468b3..34e6b18e7074 100644 --- a/app/scripts/lib/tx-verification/tx-verification-middleware.ts +++ b/app/scripts/lib/tx-verification/tx-verification-middleware.ts @@ -1,54 +1,104 @@ import { hashMessage } from '@ethersproject/hash'; import { verifyMessage } from '@ethersproject/wallet'; +import type { NetworkController } from '@metamask/network-controller'; +import { rpcErrors } from '@metamask/rpc-errors'; +import { Json, JsonRpcParams, hasProperty, isObject } from '@metamask/utils'; import { - Json, - JsonRpcParams, JsonRpcRequest, JsonRpcResponse, -} from '@metamask/utils'; -import { JsonRpcEngineEndCallback, JsonRpcEngineNextCallback, } from 'json-rpc-engine'; import { SIG_LEN, TRUSTED_BRIDGE_SIGNER } from '../../../../shared/constants/bridge'; import { FIRST_PARTY_CONTRACT_NAMES } from '../../../../shared/constants/first-party-contracts'; -export function txVerificationMiddleware( - req: JsonRpcRequest, - _res: JsonRpcResponse, - next: JsonRpcEngineNextCallback, - end: JsonRpcEngineEndCallback, +type TxParams = { + chainId?: `0x${string}`; + data: string; + from: string; + to: string; + value: string; +}; + +/** + * Creates a middleware function that verifies bridge transactions from the + * Portfolio. + * + * @param networkController - The network controller instance. + * @returns The middleware function. + */ +export function createTxVerificationMiddleware( + networkController: NetworkController, ) { - // ignore if not sendTransaction and if the params not an array - if (req.method !== 'eth_sendTransaction' || !Array.isArray(req.params)) { + return function txVerificationMiddleware( + req: JsonRpcRequest, + _res: JsonRpcResponse, + next: JsonRpcEngineNextCallback, + end: JsonRpcEngineEndCallback, + ) { + if ( + req.method !== 'eth_sendTransaction' || + !Array.isArray(req.params) || + !isValidParams(req.params) + ) { + return next(); + } + + // the tx object is the first element + const params = req.params[0]; + + const chainId = + typeof params.chainId === 'string' + ? (params.chainId.toLowerCase() as `0x${string}`) + : networkController.state.providerConfig.chainId; + + // if the recipient address is not the bridge contract, skip verification + if ( + params.to.toLowerCase() !== + FIRST_PARTY_CONTRACT_NAMES['MetaMask Bridge'][chainId].toLowerCase() + ) { + return next(); + } + + const paramsToVerify = { + to: hashMessage(params.to.toLowerCase()), + from: hashMessage(params.from.toLowerCase()), + data: hashMessage( + params.data.toLowerCase().substring(0, params.data.length - SIG_LEN), + ), + value: hashMessage(params.value.toLowerCase()), + }; + const h = hashMessage(JSON.stringify(paramsToVerify)); + + // signature is 130 chars in length at the end + const signature = `0x${params.data.substring(-SIG_LEN)}`; + const addressToVerify = verifyMessage(h, signature); + + if (addressToVerify.toLowerCase() !== TRUSTED_BRIDGE_SIGNER.toLowerCase()) { + return end( + rpcErrors.invalidParams('Invalid bridge transaction signature.'), + ); + } return next(); - } - - // 0 tx object is the first element - const params = req.params[0]; - const paramsToVerify = { - to: hashMessage(params.to.toLowerCase()), - from: hashMessage(params.from.toLowerCase()), - data: hashMessage( - // strip signature from data - params.data.toLowerCase().substr(0, params.data.length - SIG_LEN), - ), - value: hashMessage(params.value.toLowerCase()), }; - const h = hashMessage(JSON.stringify(paramsToVerify)); - // signature is 130 chars in length at the end - const signature = `0x${params.data.substr(-SIG_LEN)}`; - const addressToVerify = verifyMessage(h, signature); - const canSubmit = - params.to.toLowerCase() === - FIRST_PARTY_CONTRACT_NAMES['MetaMask Bridge'][params.chainId].toLowerCase() - ? addressToVerify.toLowerCase() === TRUSTED_BRIDGE_SIGNER.toLowerCase() - : true; - - if (!canSubmit) { - end(new Error('Validation Error')); - } - - // successful validation - return next(); +} + +/** + * Checks if the params of a JSON-RPC request are valid `eth_sendTransaction` + * params. + * + * @param params - The params to validate. + * @returns Whether the params are valid. + */ +function isValidParams(params: Json[]): params is [TxParams] { + return ( + isObject(params[0]) && + (!hasProperty(params[0], 'chainId') || + (typeof params[0].chainId === 'string' && + params[0].chainId.startsWith('0x'))) && + typeof params[0].data === 'string' && + typeof params[0].from === 'string' && + typeof params[0].to === 'string' && + typeof params[0].value === 'string' + ); } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index c3d1b6374d67..b2f8d0d99ed9 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -219,6 +219,7 @@ import { getSmartTransactionsOptInStatus, getCurrentChainSupportsSmartTransactions, } from '../../shared/modules/selectors'; +import { BaseUrl } from '../../shared/constants/urls'; import { ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) handleMMITransactionUpdate, @@ -329,7 +330,7 @@ import AuthenticationController from './controllers/authentication/authenticatio import UserStorageController from './controllers/user-storage/user-storage-controller'; import { PushPlatformNotificationsController } from './controllers/push-platform-notifications/push-platform-notifications'; import { MetamaskNotificationsController } from './controllers/metamask-notifications/metamask-notifications'; -import { txVerificationMiddleware } from './lib/tx-verification/tx-verification-middleware'; +import { createTxVerificationMiddleware } from './lib/tx-verification/tx-verification-middleware'; import { updateSecurityAlertResponse } from './lib/ppom/ppom-util'; export const METAMASK_CONTROLLER_EVENTS = { @@ -5119,8 +5120,8 @@ export default class MetamaskController extends EventEmitter { engine.push(createLoggerMiddleware({ origin })); engine.push(this.permissionLogController.createMiddleware()); - if (origin === 'https://portfolio.metamask.io' || 'http://localhost:3000') { - engine.push(txVerificationMiddleware); + if (origin === BaseUrl.Portfolio) { + engine.push(createTxVerificationMiddleware(this.networkController)); } ///: BEGIN:ONLY_INCLUDE_IF(blockaid) diff --git a/package.json b/package.json index e11a4150359a..9f375e59f055 100644 --- a/package.json +++ b/package.json @@ -328,6 +328,7 @@ "@metamask/providers": "^14.0.2", "@metamask/queued-request-controller": "^0.10.0", "@metamask/rate-limit-controller": "^5.0.1", + "@metamask/rpc-errors": "^6.2.1", "@metamask/safe-event-emitter": "^3.1.1", "@metamask/scure-bip39": "^2.0.3", "@metamask/selected-network-controller": "^13.0.0", diff --git a/yarn.lock b/yarn.lock index 164c81e21b4a..1bf391473ce0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -24927,6 +24927,7 @@ __metadata: "@metamask/providers": "npm:^14.0.2" "@metamask/queued-request-controller": "npm:^0.10.0" "@metamask/rate-limit-controller": "npm:^5.0.1" + "@metamask/rpc-errors": "npm:^6.2.1" "@metamask/safe-event-emitter": "npm:^3.1.1" "@metamask/scure-bip39": "npm:^2.0.3" "@metamask/selected-network-controller": "npm:^13.0.0"