# MixinTransfer.sol#transferFrom Wrong implementation can potentially allows attackers to reverse transfer and cause fund loss to the users

Unlock Protocol transferFrom fails to reset key manager when recipient has no key, letting sender reclaim transferred key and reverse payment.

- Payout: Audit contest
- Severity: high
- Program: Unlock Protocol
- Found by: WatchPug
- Disclosed: 2022-04-20
- Write-up quality: 6/10
- Original: https://github.com/code-423n4/2021-11-unlock-findings/issues/182
- Library page: https://bugbountylibrary.com/r/c4%3A2021-11-unlock%3AH-03
- Tags: area:web3 domain:web3 impact:funds-loss kind:contest platform:code4rena sev:high tech:solidity technique:source-review vuln:access-control vuln:business-logic

**Target:** MixinTransfer.sol transferFrom (ERC721-like key NFT)
**Root cause:** In the branch handling a recipient with no existing key (toKey.tokenId == 0), _setKeyManagerOf is not called, so the sender's key manager role persists after transfer.
**Steps:**
1. Alice owns a valid key and calls `setKeyManagerOf()` to make herself key manager
2. Alice calls `transferFrom()` to transfer the key to Bob, who pays Alice
3. Alice, still key manager, calls `transferFrom()` again to transfer the key back from Bob
4. Bob loses the key and his payment
**How found:** Code review of the transferFrom branches: noticed the no-key branch (L131-136) omits the _setKeyManagerOf reset that the expired-key branch performs.
**Exploitation:** Attacker sells a key via transferFrom to a buyer, retains key manager role due to missing reset, then uses that role to transfer the key back to herself, keeping the payment.
**Insight:** When a function has parallel branches for 'no existing token' vs 'expired token', diff the branches for state resets (approvals, managers, roles) present in one but missing in the other.