[Bug]: Signatures - Verify 3rd party details disappears if an empty domain is passed on a Typed Signature 3 & 4
Describe the bug
Problem: whenever we are triggering a Typed Signature from a dapp, if the dapp passes an empty domain, we can see how the Verify third party details section disappears completely.
Furthermore, it is displayed as a Typed 4 signature even though this message does not follow the specs (see below)
Expected behavior
- From the specs: dapps should pass at least 1 value in the
domainfield, meaning we should either indicate this somehow like a warning, or directly not parsing it as Typed 4?
the type of eip712Domain is a struct named EIP712Domain with one or more of the below fields. Protocol designers only need to include the fields that make sense for their signing domain. Unused fields are left out of the struct type.
- string name the user readable name of signing domain, i.e. the name of the DApp or the protocol.
- string version the current major version of the signing domain. Signatures from different versions are not compatible.
- uint256 chainId the [EIP-155](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md) chain id. The user-agent should refuse signing if it does not match the currently active chain.
- address verifyingContract the address of the contract that will verify the signature. The user-agent may do contract specific phishing prevention.
- bytes32 salt an disambiguating salt for the protocol. This can be used as a domain separator of last resort.
- We should always display the
Verify 3rd party detailsand indicate if the dapp hasn't provided the verifying contract address?
Screenshots
https://github.com/MetaMask/metamask-extension/assets/54408225/47a70e53-15d5-4c96-bb8b-908812b9fd48
Steps to reproduce
- Checkout test dapp project
- Remove the domain data from the Typed Signature 3 oand 4 section
- Start test dapp
- Click Sign Typed 4
- See Verify 3rd party details is not there anymore
Error messages or log output
No response
Version
11.1.0 (possibly earlier)
Build type
None
Browser
Chrome
Operating system
Linux
Hardware wallet
No response
Additional context
No response
Severity
No response
Should we be validating if the requests meets the spec on the API level?
the json-schema in the spec is pretty lenient on this
it has domain as required but only that domain is an object
required: ["domain",
domain: {type: 'object'},
but since we are actually using domain we should probably specify this deeper.
It should be:
"domain": {
"type": "object",
"properties": {
"chainId": {
"$ref": "#/components/schemas/ChainId"
},
"name": {
"type": "string"
},
"verifyingContract": {
"$ref": "#/components/schemas/Address"
},
"version": {
"type": "string",
"const": "1"
},
}
}
Should we be validating if the requests meets the spec on the API level?
this is definitely something we want to do/should do. I know for this particular case we actually have just copied in the schema right into eth-sig-utils and use a json schema validator, but ideally it should get it from the api-specs and do it at the api level.
We will add a small note to the https://docs.metamask.io/wallet/reference/eth_signtypeddata_v4/ documentation to clarify how our implementation works.
This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.