MetaMask / MetaMask/metamask-extension

Improve address validation in useSignatureMetrics

Open
#31,228 0 comments 0 reactions 0 assignees View on GitHub
external-contributor INVALID-ISSUE-TEMPLATE Sev2-normal team-core-platform
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.