fix: only scan send payees for address poisoning - #9943
Draft
adonesky1 wants to merge 2 commits into
Draft
Conversation
Confirmed approves, swaps, and contract calls were adding token and protocol addresses to the known-recipient set, which made vanity token contracts look like poisoning matches. Only hydrate and compare against user-chosen send payees.
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Restrict address poisoning known recipients to user-chosen send payees so confirmed approves, swaps, and contract calls stop putting token and protocol addresses into the comparison set.
Summary
Export
getSendRecipientsand hydrate the phishing known-recipient set from it instead ofgetEffectiveRecipient, which falls back totxParams.tofor every non-transfer type.Problem
Address poisoning is a payee mixup: the user previously sent to Alice, an attacker grinds a lookalike EOA, and the user pastes from history. We were also storing token, Permit2, router, and other contract
tovalues from confirmed approves and contract interactions. Vanity token factories (shared prefix plus suffix) then 4+4-matched each other, which is what fired on the o1.exchange Permit2 approve (dogue vs another B20 token).getEffectiveRecipientis the right helper for first-time interaction (the contracttois who we are calling). It is the wrong helper for poisoning.Solution
Add
getSendRecipientsnext togetEffectiveRecipientand use it only for poisoning hydration.Included:
simpleSendto, preferringtxParamsOriginalwhen presentto/_tofor ERC-20/721/1155 transfer methods (no fallback to the token contract)swapAndSendRecipientNot included: approves, Permit2, swaps, bridges, staking, and other contract interactions.
getEffectiveRecipientis unchanged so first-time interaction keeps using the contractto.Follow-up client PR: MetaMask/metamask-extension#45724
Mobile confirmations already type-gate; they pick up the known-set fix when this phishing-controller version is bumped.
Risk
References
Checklist