Add validation of suggested metadata on ERC20 wallet_watchAsset confirmation
#1207
Assignees
Comments
3 tasks
bergeron
added a commit
that referenced
this issue
Dec 11, 2023
## Explanation ### Problem: `wallet_watchAsset` accepts `type`, `address`, `symbol`, and `decimals` from dapps. But it does not verify the accuracy of this information against the chain: - You can pass `type=ERC20`, but provide an `address` that is an EOA, `ERC721`, or `ERC1155` contract. - You can pass an `ERC20` contract, but with the wrong `symbol` or `decimals`. Malicious dapps can watch scam tokens under legit symbols, as seen here: https://github.com/MetaMask/core/assets/3500406/55a294ab-e3af-409b-814f-e61e74653f99 ### Solution: Query the chain to get the contract type, `symbol`, and `decimals`. Validate the request against the on-chain data. If `symbol` and `decimals` are defined in the contract (they're optional in ERC20), then the caller may now omit them in the request to `wallet_watchAsset` . Otherwise they will be validated against the on-chain data. ## References - #1207 - https://github.com/ethereum/EIPs/blob/master/EIPS/eip-747.md ## Changelog <!-- If you're making any consumer-facing changes, list those changes here as if you were updating a changelog, using the template below as a guide. (CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or FIXED. For security-related issues, follow the Security Advisory process.) Please take care to name the exact pieces of the API you've added or changed (e.g. types, interfaces, functions, or methods). If there are any breaking changes, make sure to offer a solution for consumers to follow once they upgrade to the changes. Finally, if you're only making changes to development scripts or tests, you may replace the template below with "None". --> ### `@metamask/assets-controllers` - **BREAKING**: `TokensController.watchAsset` now performs on-chain validation of the asset's symbol and decimals, if they're defined in the contract. If the symbol and decimals are defined in the contract, they are no longer required to be passed to `watchAsset` ## 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 - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate --------- Co-authored-by: Alex Donesky <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
This issue aims to improve the reliability of suggested metadata for ERC20 tokens in the MetaMask extension by validating it against the metadata fetched directly from the chain during the wallet_watchAsset confirmation process.
Expected behavior
When the wallet_watchAsset function is invoked, the extension should compare the suggested metadata with the on-chain metadata before displaying the confirmation dialog. This ensures the provided metadata is accurate and trustworthy.
Proposed solution
Update the wallet_watchAsset function to fetch token metadata directly from the blockchain, such as token name, symbol, and decimals.
Compare the fetched metadata with the suggested metadata. If discrepancies are found, either display an error message or just use the on-chain metadata.
Potential drawbacks
This approach may introduce additional latency during the wallet_watchAsset process due to blockchain data fetching. However, the benefits of providing accurate token metadata outweigh the potential drawbacks.
The text was updated successfully, but these errors were encountered: