# Approvals not cleared after key transfer

Unlock Protocol transferFrom fails to clear single-token approvals when recipient's key expired and key manager already zero, letting a prior approver reclaim a sold key.

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

**Target:** ERC721-like key NFT transfer logic (MixinTransfer/MixinKeys)
**Root cause:** transferFrom only calls _clearApproval when the recipient had no key; in the expired-key branch it calls _setKeyManagerOf, which skips _clearApproval when keyManagerOf is already address(0), leaving approved[tokenId] stale.
**Steps:**
1. Attacker A owns valuable key tokenId=42 with far-future expiry.
2. A calls `setApprovalForAll(A', true)` setting managerToOperatorApproved[A][A'].
3. A transfers key to a fresh account to invoke `_setKeyManagerOf(42, address(0))`, clearing key manager.
4. A calls `approve(A', 42)` setting `approved[42]=A'`.
5. A sells token 42 to victim V who previously held an expired key; `transferFrom(A,V,42)` hits expired branch, `_setKeyManagerOf(42,0)` no-ops since already 0, so `approved[42]` stays A'.
6. A' calls `transferFrom(V,A',42)`; `onlyKeyManagerOrApproved` passes via stale `approved[42]`, reclaiming the key.
**How found:** Code review of the three approval types and the transferFrom branches; noticed _setKeyManagerOf only clears approvals when the manager value actually changes, so the expired-key path leaves approved[] stale.
**Exploitation:** Seller sets operator and single-token approvals, sells the key at a discount to a victim whose prior key expired, then the approved attacker account transfers the key back from the victim, stealing the sold asset.
**Insight:** When a contract has multiple parallel approval maps, audit every state-transition branch for which maps get cleared; a guard like `if(old != new)` inside a clear-helper silently skips cleanup when the value is already the target.