feat: validate eth_sendTransaction / eth_signTransaction params - #9482
Conversation
| } from './methods/wallet-get-supported-execution-permissions'; | ||
| export * from './providerAsMiddleware'; | ||
| export * from './retryOnEmpty'; | ||
| export { validateTransactionParams } from './utils/validation'; |
There was a problem hiding this comment.
Extension/mobile PPOM middleware can gate params before their own normalizeTransactionParams call (which is where the WASM crash actually originates in clients).
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
mcmire
left a comment
There was a problem hiding this comment.
This looks good to me, but I would like another review from my team just to be safe.
| */ | ||
| export function validateTransactionParams(params: unknown): void { | ||
| if (Array.isArray(params)) { | ||
| throw rpcErrors.invalidInput(); |
There was a problem hiding this comment.
I believe we use invalidParams elsewhere when parameters have an unexpected shape
There was a problem hiding this comment.
In fact, we don't need this at all as we are validating vs TransactionParamsStruct so removed this.
There was a problem hiding this comment.
Also here, I wouldn't recommend we change any errors or messages at this stage if possible, since the validation we'd like to add is very limited.
Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
## Explanation Release `1169.0.0` with version bumps for: - **`@metamask/eth-json-rpc-middleware`** `23.1.3` → `24.0.0` (major) - **`@metamask/network-controller`** `35.0.0` → `35.0.1` (patch) ### `@metamask/eth-json-rpc-middleware@24.0.0` **Breaking:** Add strict validation for `eth_sendTransaction` and `eth_signTransaction` params ([MetaMask#9482](MetaMask#9482)) - Reject requests whose params do not match the transaction schema (extraneous top-level keys, ill-typed fields such as non-hex `to`/`data`, malformed `accessList` / `authorizationList` entries) or exceed `MAX_TRANSACTION_PARAMS_SIZE_BYTES` when serialized - Prevents downstream normalization / PPOM WASM from crashing on deeply-nested junk fields or padded payloads and silently bypassing security scans Other changes: - Bump `@metamask/utils` from `^11.9.0` to `^11.11.0` ([MetaMask#9074](MetaMask#9074)) - Bump `@metamask/json-rpc-engine` from `^10.2.4` to `^10.5.0` ([MetaMask#8661](MetaMask#8661), [MetaMask#8746](MetaMask#8746), [MetaMask#8753](MetaMask#8753)) - Bump `@metamask/message-manager` from `^14.1.1` to `^14.1.2` ([MetaMask#8755](MetaMask#8755)) - Drop `pify` dependency, which was no longer used in source ([MetaMask#9064](MetaMask#9064)) ### `@metamask/network-controller@35.0.1` - Bump `@metamask/eth-json-rpc-middleware` from `^23.1.3` to `^24.0.0` ([MetaMask#9758](MetaMask#9758)) ## References - [MetaMask#9482](MetaMask#9482) — feat: validate `eth_sendTransaction` / `eth_signTransaction` params ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > The release propagates a major middleware bump that rejects previously accepted malformed transaction RPC params; integrators on strict validation paths should expect possible breakage for non-conformant dapps, though this PR itself only changes versions and docs. > > **Overview** > This PR cuts **monorepo release `1169.0.0`** by versioning packages and aligning dependents—no application source changes beyond manifests and changelogs. > > **`@metamask/eth-json-rpc-middleware@24.0.0`** is published with changelog release notes for the existing **breaking** strict validation on `eth_sendTransaction` / `eth_signTransaction` (schema, size limits, rejection of malformed or oversized params). > > **`@metamask/network-controller@35.0.1`** bumps its dependency on that middleware from `^23.1.3` to `^24.0.0`. Root and many workspace packages update `@metamask/network-controller` to `^35.0.1`, with matching **Unreleased** changelog lines and **`yarn.lock`** resolution updates. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 455c03b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: cryptodev-2s <109512101+cryptodev-2s@users.noreply.github.com>
Explanation
Malicious dapps can send
eth_sendTransactionwith a valid-looking payload plus a deeply-nested junk field (e.g.test.b.b.b...~1200 levels). Downstream normalization in@metamask/transaction-controllerrecurses into these params and throwsRangeError: Maximum call stack size exceeded. When this crash happens inside the extension/mobile PPOM middleware it short-circuits the security-scan pipeline: the request never reaches the Security Alerts API, and the user sees the confirmation without a Blockaid scan.The same class of attack was fixed for
eth_signTypedData_v4in #8526 by rejecting requests with extraneous top-level keys. This PR applies the equivalent guardrail to transaction methods.Changes
validateTransactionParams(params)insrc/utils/validation.ts, exported from the package. Rejects withrpcErrors.invalidInput()when:paramsis not a plain object, orTransactionParams(accessList,authorizationList,chainId,data,from,gas,gasLimit,gasPrice,maxFeePerGas,maxPriorityFeePerGas,nonce,to,type,value), oraccessList/authorizationList, are ≤4 levels deep).eth_sendTransactionandeth_signTransactionhandlers increateWalletMiddleware, beforevalidateAndNormalizeKeyholder.index.tsso extension/mobile PPOM middleware can gate params before their ownnormalizeTransactionParamscall (which is where the WASM crash actually originates in clients).Design notes
invalidInput. Stripping silently would hide the attack pattern from telemetry.TransactionParams(estimateGasError,estimatedBaseFee,estimateSuggested,estimateUsed,gasUsed) — dapps should not be able to inject those.References
Checklist
Note
High Risk
Breaking RPC validation on security-critical transaction paths may reject previously accepted dapp payloads; changes directly target bypass of Blockaid/PPOM scanning.
Overview
BREAKING:
eth_sendTransactionandeth_signTransactionnow run newvalidateTransactionParamsbefore address checks. Requests must match a fixed transaction field schema (no extra top-level keys, typedaccessList/authorizationList, quantity fields as hex string or number) and stay under 200 KB serialized size or they fail with invalid params.The helper is exported from the package so clients can reuse the same guard ahead of normalization/PPOM. This mirrors the earlier
eth_signTypedData_v4hardening and blocks junk nested fields and padded payloads that could crash downstream normalization and skip security scans.Reviewed by Cursor Bugbot for commit a4e603a. Bugbot is set up for automated code reviews on this repo. Configure here.