From c66e89916f79b8d01bb05cc81d1f26566a66e6c9 Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Fri, 26 Jul 2024 16:59:53 -0500 Subject: [PATCH 1/5] remove eth_sign (#320) --- src/wallet.ts | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/src/wallet.ts b/src/wallet.ts index db8eee6c..0226d745 100644 --- a/src/wallet.ts +++ b/src/wallet.ts @@ -55,10 +55,6 @@ export interface WalletMiddlewareOptions { address: string, req: JsonRpcRequest, ) => Promise; - processEthSignMessage?: ( - msgParams: MessageParams, - req: JsonRpcRequest, - ) => Promise; processPersonalMessage?: ( msgParams: MessageParams, req: JsonRpcRequest, @@ -92,7 +88,6 @@ export function createWalletMiddleware({ getAccounts, processDecryptMessage, processEncryptionPublicKey, - processEthSignMessage, processPersonalMessage, processTransaction, processSignTransaction, @@ -113,7 +108,6 @@ WalletMiddlewareOptions): JsonRpcMiddleware { eth_sendTransaction: createAsyncMiddleware(sendTransaction), eth_signTransaction: createAsyncMiddleware(signTransaction), // message signatures - eth_sign: createAsyncMiddleware(ethSign), eth_signTypedData: createAsyncMiddleware(signTypedData), eth_signTypedData_v3: createAsyncMiddleware(signTypedDataV3), eth_signTypedData_v4: createAsyncMiddleware(signTypedDataV4), @@ -195,36 +189,6 @@ WalletMiddlewareOptions): JsonRpcMiddleware { // // message signatures // - - async function ethSign( - req: JsonRpcRequest, - res: PendingJsonRpcResponse, - ): Promise { - if (!processEthSignMessage) { - throw rpcErrors.methodNotSupported(); - } - if ( - !req?.params || - !Array.isArray(req.params) || - !(req.params.length >= 2) - ) { - throw rpcErrors.invalidInput(); - } - - const params = req.params as [string, string, Record?]; - const address: string = await validateAndNormalizeKeyholder(params[0], req); - const message = params[1]; - const extraParams = params[2] || {}; - const msgParams: MessageParams = { - ...extraParams, - from: address, - data: message, - signatureMethod: 'eth_sign', - }; - - res.result = await processEthSignMessage(msgParams, req); - } - async function signTypedData( req: JsonRpcRequest, res: PendingJsonRpcResponse, From 17d1ac92d71280b4ba6f4baeedde90c2cea55043 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 20 Aug 2024 11:14:35 -0400 Subject: [PATCH 2/5] Add changelog entries for `#318` (#327) --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e45bcd5..927bb61b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/rpc-errors` from `^6.0.0` to `^6.3.1` ([#323](https://github.com/MetaMask/eth-json-rpc-middleware/pull/323)) - Bump `@metamask/utils` from `^8.1.0` to `^9.1.0` ([#323](https://github.com/MetaMask/eth-json-rpc-middleware/pull/323)) +### Security +- **BREAKING:** Typed signature validation only replaces `0X` prefix with `0x`, and contract address normalization is removed for decimal and octal values ([#318](https://github.com/MetaMask/eth-json-rpc-middleware/pull/318)) + - Threat actors have been manipulating `eth_signTypedData_v4` fields to cause failures in blockaid's detectors. + - Extension crashes with an error when performing Malicious permit with a non-0x prefixed integer address. + - This fixes an issue where the key value row or petname component disappears if a signed address is prefixed by "0X" instead of "0x". + ## [13.0.0] ### Changed - **BREAKING**: Drop support for Node.js v16; add support for Node.js v20, v22 ([#312](https://github.com/MetaMask/eth-json-rpc-middleware/pull/312)) From 25c0bed1730f1f3093f8c264a6d9595a64d0d77f Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Wed, 4 Sep 2024 22:37:30 +0530 Subject: [PATCH 3/5] Request validation should not throw if verifyingContract is not defined in typed signature (#328) --- src/wallet.test.ts | 73 ++++++++++++++++++++++++++++++++++++++++++++++ src/wallet.ts | 6 ++-- 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/src/wallet.test.ts b/src/wallet.test.ts index 9a8e783c..a58efb81 100644 --- a/src/wallet.test.ts +++ b/src/wallet.test.ts @@ -432,6 +432,50 @@ describe('wallet', () => { const promise = pify(engine.handle).call(engine, payload); await expect(promise).rejects.toThrow('Invalid input.'); }); + + it('should not throw if verifyingContract is undefined', async () => { + const { engine } = createTestSetup(); + const getAccounts = async () => testAddresses.slice(); + const witnessedMsgParams: TypedMessageParams[] = []; + const processTypedMessageV3 = async (msgParams: TypedMessageParams) => { + witnessedMsgParams.push(msgParams); + // Assume testMsgSig is the expected signature result + return testMsgSig; + }; + + engine.push( + createWalletMiddleware({ getAccounts, processTypedMessageV3 }), + ); + + const message = { + types: { + EIP712Domain: [ + { name: 'name', type: 'string' }, + { name: 'version', type: 'string' }, + { name: 'chainId', type: 'uint256' }, + { name: 'verifyingContract', type: 'address' }, + ], + }, + primaryType: 'EIP712Domain', + message: {}, + }; + + const stringifiedMessage = JSON.stringify(message); + + const payload = { + method: 'eth_signTypedData_v3', + params: [testAddresses[0], stringifiedMessage], // Assuming testAddresses[0] is a valid address from your setup + }; + + const promise = pify(engine.handle).call(engine, payload); + const result = await promise; + expect(result).toStrictEqual({ + id: undefined, + jsonrpc: undefined, + result: + '0x68dc980608bceb5f99f691e62c32caccaee05317309015e9454eba1a14c3cd4505d1dd098b8339801239c9bcaac3c4df95569dcf307108b92f68711379be14d81c', + }); + }); }); describe('signTypedDataV4', () => { @@ -524,6 +568,35 @@ describe('wallet', () => { const promise = pify(engine.handle).call(engine, payload); await expect(promise).rejects.toThrow('Invalid input.'); }); + + it('should not throw if request is permit with undefined value for verifyingContract address', async () => { + const { engine } = createTestSetup(); + const getAccounts = async () => testAddresses.slice(); + const witnessedMsgParams: TypedMessageParams[] = []; + const processTypedMessageV4 = async (msgParams: TypedMessageParams) => { + witnessedMsgParams.push(msgParams); + // Assume testMsgSig is the expected signature result + return testMsgSig; + }; + + engine.push( + createWalletMiddleware({ getAccounts, processTypedMessageV4 }), + ); + + const payload = { + method: 'eth_signTypedData_v4', + params: [testAddresses[0], JSON.stringify(getMsgParams())], + }; + + const promise = pify(engine.handle).call(engine, payload); + const result = await promise; + expect(result).toStrictEqual({ + id: undefined, + jsonrpc: undefined, + result: + '0x68dc980608bceb5f99f691e62c32caccaee05317309015e9454eba1a14c3cd4505d1dd098b8339801239c9bcaac3c4df95569dcf307108b92f68711379be14d81c', + }); + }); }); describe('sign', () => { diff --git a/src/wallet.ts b/src/wallet.ts index 0226d745..a1e65613 100644 --- a/src/wallet.ts +++ b/src/wallet.ts @@ -463,10 +463,8 @@ WalletMiddlewareOptions): JsonRpcMiddleware { * @param data - The data passed in typedSign request. */ function validateVerifyingContract(data: string) { - const { - domain: { verifyingContract }, - } = parseTypedMessage(data); - if (!isValidHexAddress(verifyingContract)) { + const { domain: { verifyingContract } = {} } = parseTypedMessage(data); + if (verifyingContract && !isValidHexAddress(verifyingContract)) { throw rpcErrors.invalidInput(); } } From 1d17a1d802ce83da600021c06ea0275e7ae859a0 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 6 Sep 2024 14:53:27 -0230 Subject: [PATCH 4/5] Update `main` with changes from v14.0.1 (#332) * Request validation should not throw if verifyingContract is not defined in typed signature (#328) (#330) * 14.0.1 (#331) --------- Co-authored-by: Jyoti Puri --- CHANGELOG.md | 7 ++++++- package.json | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 927bb61b..11db7260 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [14.0.1] +### Fixed +- Request validation should not throw if verifyingContract is not defined in typed signature ([#328](https://github.com/MetaMask/eth-json-rpc-middleware/pull/328)) + ## [14.0.0] ### Changed - **BREAKING:** Adapt to EIP-1193 provider changes by replacing the deprecated `sendAsync` method with the `request` method ([#317](https://github.com/MetaMask/eth-json-rpc-middleware/pull/317)) @@ -202,7 +206,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `json-rpc-engine@5.3.0` ([#53](https://github.com/MetaMask/eth-json-rpc-middleware/pull/53)) - `eth-rpc-errors@3.0.0` ([#55](https://github.com/MetaMask/eth-json-rpc-middleware/pull/55)) -[Unreleased]: https://github.com/MetaMask/eth-json-rpc-middleware/compare/v14.0.0...HEAD +[Unreleased]: https://github.com/MetaMask/eth-json-rpc-middleware/compare/v14.0.1...HEAD +[14.0.1]: https://github.com/MetaMask/eth-json-rpc-middleware/compare/v14.0.0...v14.0.1 [14.0.0]: https://github.com/MetaMask/eth-json-rpc-middleware/compare/v13.0.0...v14.0.0 [13.0.0]: https://github.com/MetaMask/eth-json-rpc-middleware/compare/v12.1.2...v13.0.0 [12.1.2]: https://github.com/MetaMask/eth-json-rpc-middleware/compare/v12.1.1...v12.1.2 diff --git a/package.json b/package.json index dbe4e495..54125b6b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/eth-json-rpc-middleware", - "version": "14.0.0", + "version": "14.0.1", "description": "Ethereum-related json-rpc-engine middleware.", "repository": { "type": "git", From 7fbac8b2b29d61cde06b91858ef6b25170af3cc8 Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Mon, 23 Sep 2024 15:52:03 +0530 Subject: [PATCH 5/5] fix: change types signatures verifyingContract validation to allow 'cosmos' as address (#334) --- src/wallet.test.ts | 29 +++++++++++++++++++++++++++++ src/wallet.ts | 8 +++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/wallet.test.ts b/src/wallet.test.ts index a58efb81..27cd1fa8 100644 --- a/src/wallet.test.ts +++ b/src/wallet.test.ts @@ -597,6 +597,35 @@ describe('wallet', () => { '0x68dc980608bceb5f99f691e62c32caccaee05317309015e9454eba1a14c3cd4505d1dd098b8339801239c9bcaac3c4df95569dcf307108b92f68711379be14d81c', }); }); + + it('should not throw if request is permit with verifyingContract address equal to "cosmos"', async () => { + const { engine } = createTestSetup(); + const getAccounts = async () => testAddresses.slice(); + const witnessedMsgParams: TypedMessageParams[] = []; + const processTypedMessageV4 = async (msgParams: TypedMessageParams) => { + witnessedMsgParams.push(msgParams); + // Assume testMsgSig is the expected signature result + return testMsgSig; + }; + + engine.push( + createWalletMiddleware({ getAccounts, processTypedMessageV4 }), + ); + + const payload = { + method: 'eth_signTypedData_v4', + params: [testAddresses[0], JSON.stringify(getMsgParams('cosmos'))], + }; + + const promise = pify(engine.handle).call(engine, payload); + const result = await promise; + expect(result).toStrictEqual({ + id: undefined, + jsonrpc: undefined, + result: + '0x68dc980608bceb5f99f691e62c32caccaee05317309015e9454eba1a14c3cd4505d1dd098b8339801239c9bcaac3c4df95569dcf307108b92f68711379be14d81c', + }); + }); }); describe('sign', () => { diff --git a/src/wallet.ts b/src/wallet.ts index a1e65613..d78e17e1 100644 --- a/src/wallet.ts +++ b/src/wallet.ts @@ -464,7 +464,13 @@ WalletMiddlewareOptions): JsonRpcMiddleware { */ function validateVerifyingContract(data: string) { const { domain: { verifyingContract } = {} } = parseTypedMessage(data); - if (verifyingContract && !isValidHexAddress(verifyingContract)) { + // Explicit check for cosmos here has been added to address this issue + // https://github.com/MetaMask/eth-json-rpc-middleware/issues/new + if ( + verifyingContract && + (verifyingContract as string) !== 'cosmos' && + !isValidHexAddress(verifyingContract) + ) { throw rpcErrors.invalidInput(); } }