# VotingEscrow's merge and withdraw aren't available for approved users

VotingEscrow._burn passes msg.sender instead of owner to _removeTokenFrom, so approved non-owners can never merge or withdraw their delegated NFTs.

- Payout: Contest reward
- Severity: high
- Program: Velodrome Finance
- Credited: hyh
- Disclosed: 2022-08-08
- Write-up quality: 6/10
- Original: https://github.com/code-423n4/2022-05-velodrome-findings/issues/66
- Library page: https://bugbountylibrary.com/r/c4%3A2022-05-velodrome%3AH-02
- Tags: domain:web3 impact:dos kind:contest platform:code4rena sev:high tech:solidity technique:source-review vuln:access-control vuln:business-logic

**Target:** VotingEscrow.sol NFT lock contract (merge/withdraw/_burn)
**Root cause:** _burn() calls _removeTokenFrom(msg.sender, _tokenId), but _removeTokenFrom asserts idToOwner[_tokenId] == _from; an approved non-owner sender fails the assert even though _isApprovedOrOwner passed.
**Steps:**
1. Owner approves account A on tokenId via approve(A, tokenId).
2. A calls merge(_from, _to) or withdraw(_tokenId).
3. _isApprovedOrOwner(msg.sender, _tokenId) returns true, so execution continues.
4. _burn(_tokenId) calls _removeTokenFrom(msg.sender, _tokenId).
5. assert(idToOwner[_tokenId] == _from) fails because _from is A, not the owner; call reverts.
6. Approved user can never merge or withdraw the NFT.
**How found:** Code reading of _burn/_removeTokenFrom: noticed _burn validates _isApprovedOrOwner but then passes msg.sender as the owner argument to _removeTokenFrom, which asserts ownership.
**Insight:** When a function checks _isApprovedOrOwner but later mutates owner-indexed accounting, verify it uses ownerOf(_tokenId) rather than msg.sender; delegated-caller paths are a common source of permanent DoS on NFT-style contracts.