MetaMask / MetaMask/metamask-extension
Improve address validation in useSignatureMetrics
- Dominant language
- TypeScript
- Stars
- 13.2k
- Forks
- 5.6k
- Avg merge
- 2d 5h
- Merged PRs (30d)
- 451
Description
Our usage of `getAddressAccountType` utility [doesn't throw invalid address](https://github.com/MetaMask/metamask-mobile/blob/main/app/util/address/index.ts#L314) and passed `messageData.from` property is already validated in multiple places in signature requests. That said we are sure of usage in `useSignatureMetrics` passing expected address even though the given error context is missing due to scrubbing rule.
There are multiple signs that this function is not working properly on other places too
[Example 1](https://metamask.sentry.io/issues/4909996853/?project=2299799&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D%20%2A%2A%20is%20not%20imported&referrer=issue-stream&sort=date&stream_index=6)
[Example 2](https://metamask.sentry.io/issues/6397388046/?project=2299799&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D%20%2A%2A%20is%20not%20imported&referrer=issue-stream&sort=date&stream_index=9)
[Example 3 ](https://metamask.sentry.io/issues/6396725637/?project=2299799&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D%20%2A%2A%20is%20not%20imported&referrer=issue-stream&sort=date&stream_index=0)
[All related issues ](https://metamask.sentry.io/issues/?project=2299799&query=is%3Aunresolved%20%2A%2A%20is%20not%20imported&referrer=issue-list&sort=date&statsPeriod=14d&viewId=default0)
This task naturally assigned us because the most encountered error case was in signatures. But I believe this should be investigated by wallet platform team (assuming they have the responsibility of `KeyringController`) to understand root cause of not matching given address with any keyring accounts.
Originally posted by @OGPoyraz at https://github.com/MetaMask/metamask-mobile/issues/13764#issuecomment-2728565868
Contributor guide
Research direction
Start with useSignatureMetrics and getAddressAccountType in app/util/address/index.ts around line 314, then trace how messageData.from is validated in signature requests. Review the KeyringController responsibility and the linked Sentry examples to identify why the address does not match a keyring account. Done means the root cause and affected ownership are established, with the required address-validation change defined.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100