MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
12.01k stars 4.91k forks source link

Bug: signTypedData_v4 fails when verifyingContract isn't a evm address #26980

Closed foker closed 1 month ago

foker commented 1 month ago

Describe the bug

We work with cosmos blockchain and some transactions in it need to be formed with signTypedData_v4 And now we can't for transaction if 'verifyingContract' field is not evm-like address (which is a common and valid situation for cosmos blockchain, with which Metamask works)

Problem with it appeared yesterday, after users got '12.1.3' update for metamask-extension We checked the same behavior in 12.0.4 and it works correctly I did a short investigation and it looks like it happened after updating version of @metamask/eth-json-rpc-middleware dependency to ^14.0.1. Which, in turn, has broken functionality with next validation:

` /**

Expected behavior

eth_signTypedData_v4 method doesn't throw if verifyingContract isn't a evm-valid address

Screenshots/Recordings

-

Steps to reproduce

Working example : (I took it from here ) : await window.ethereum.request({ "method": "eth_signTypedData_v4", "params": [ "0xcb4dd4c1605117abc9bbcf1834377d7988d8e03b", { types: { EIP712Domain: [ { name: "name", type: "string" }, { name: "version", type: "string" }, { name: "chainId", type: "uint256" }, { name: "verifyingContract", type: "address" } ], Person: [ { name: "name", type: "string" }, { name: "wallet", type: "address" } ], Mail: [ { name: "from", type: "Person" }, { name: "to", type: "Person" }, { name: "contents", type: "string" } ] }, primaryType: "Mail", domain: { name: "Ether Mail", version: "1", chainId: 1, verifyingContract: "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826" }, message: { from: { name: "Cow", wallet: "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826" }, to: { name: "Bob", wallet: "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB" }, contents: "Hello, Bob!" } } ], });

broken example I changed verifyContract to cosmos:

await window.ethereum.request({ "method": "eth_signTypedData_v4", "params": [ "0xcb4dd4c1605117abc9bbcf1834377d7988d8e03b", { types: { EIP712Domain: [ { name: "name", type: "string" }, { name: "version", type: "string" }, { name: "chainId", type: "uint256" }, { name: "verifyingContract", type: "address" } ], Person: [ { name: "name", type: "string" }, { name: "wallet", type: "address" } ], Mail: [ { name: "from", type: "Person" }, { name: "to", type: "Person" }, { name: "contents", type: "string" } ] }, primaryType: "Mail", domain: { name: "Ether Mail", version: "1", chainId: 1, verifyingContract: "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826" }, message: { from: { name: "Cow", wallet: "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826" }, to: { name: "Bob", wallet: "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB" }, contents: "Hello, Bob!" } } ], });

Error messages or log output

`buildRequest.js:35 Uncaught (in promise) InvalidInputRpcError: Missing or invalid parameters.
Double check you have provided the correct parameters.

Details: Invalid input.
Version: viem@2.9.4
    at delay.count.count (buildRequest.js:35:1)
    at async attemptRetry (withRetry.js:12:1)`

Detection stage

In production (default)

Version

12.1.3

Build type

None

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

Severity

Critical

randy75828 commented 1 month ago

Second to this! All cosmos chain using eip712 for metamask tx signing are not working because of this validation. Can someone from the team fix it?

DanielTech21 commented 1 month ago

Hi @foker

Thank you for bringing this to our attention.

We will review the issue and get back to you.

digiwand commented 1 month ago

greetings @foker,

Thanks for raising this issue! The working and broken examples look the same and work.

That said, I think I see what you mean by the provided code you shared, and it appears to be a regression.

We are investigating this further. Are you able to provide the address you used for the broken verifyingContract example as it might help with the investigation?

bschorchit commented 1 month ago

@foker @randy75828 please do share a broken example once you have a chance. We're investigating how we can revert this regression for cosmos without reverting the changes for evm addresses.

foker commented 1 month ago

Can you please try like that

await window.ethereum.request({ "method": "eth_signTypedData_v4", "params": [ "0xcb4dd4c1605117abc9bbcf1834377d7988d8e03b", { types: { EIP712Domain: [ { name: "name", type: "string" }, { name: "version", type: "string" }, { name: "chainId", type: "uint256" }, { name: "verifyingContract", type: "address" } ], Person: [ { name: "name", type: "string" }, { name: "wallet", type: "address" } ], Mail: [ { name: "from", type: "Person" }, { name: "to", type: "Person" }, { name: "contents", type: "string" } ] }, primaryType: "Mail", domain: { name: "Ether Mail", version: "1", chainId: 1, verifyingContract: "cosmos" }, message: { from: { name: "Cow", wallet: "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826" }, to: { name: "Bob", wallet: "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB" }, contents: "Hello, Bob!" } } ], });

My address is 0xcB4dd4C1605117aBc9bBcf1834377d7988d8E03b

danjm commented 1 month ago

@foker @randy75828 @mtsitrin Is there any documentation anywhere on why cosmos dapps use the verifyingContract field in this way?

randy75828 commented 1 month ago

https://github.com/evmos/ethermint/blob/main/ethereum/eip712/domain.go#L29

we integrate this library when we want to extend evm capability for cosmos chains. Even though this repo is read-only now, i believe most cosmos chains still uses this.

As far as I know, there is no documentation as to why it is cosmos. I presume it is just to satisfy the interface and therefore the library provided a dummy string for verifyingContract

mtsitrin commented 1 month ago

I've created a PR that supposed to fix this issue (untested in prod) https://github.com/MetaMask/eth-json-rpc-middleware/pull/333

danjm commented 1 month ago

@randy75828 @mtsitrin @foker We are preparing a release that should fix this issue. It applies a very similar solution to @mtsitrin's proposed PR

Would one of you be able to download a release candidate build and test that it fixes your dapp? We would like to be certain that it fixes the problem. To do so:

  1. View this github comment and click "builds ready": https://github.com/MetaMask/metamask-extension/pull/27066#issuecomment-2344300168
  2. Download the chrome build (the first in the list, next two "builds:")
  3. Open chrome://extensions
  4. Toggle on "developer mode"
  5. Toggle off your existing metamask installation
  6. Drag and drop the downloaded zip file onto the chrome://extensions page
  7. Open this metamask install, onboard, and test the functionality
  8. Once you have completed testing, remove the metamask install, toggle on your other metamask install, and toggle off "developer mode"

Alternatively, if you are unable to do that, could one of you give us step by step instructions on how we can test this on a live dapp?

mtsitrin commented 1 month ago

Hi @danjm Just tested it. It works. Tested over https://portal.dymension.xyz

Thx for the quick response

danjm commented 1 month ago

thank-you @mtsitrin!

foker commented 1 month ago

we tested on our side too, works like a charm

metamaskbot commented 1 month ago

Missing release label release-12.5.0 on issue. Adding release label release-12.5.0 on issue, as issue is linked to PR #27021 which has this release label.