Submitted by hyh, also found by hansfriese, rotcivegaf, and WatchPug
Users who are approved, but do not own a particular NFT, are supposed to be eligible to call merge and withdraw from the NFT.
Currently _burn(), used by merge() and withdraw() to remove the NFT from the system, will revert unless the sender is the owner of NFT as the function tries to update the accounting for the sender, not the owner.
Setting the severity to medium as the impact is merge() and withdraw() permanent unavailability for any approved sender, who isnt the owner of the involved NFT.
_removeTokenFrom() requires _from to be the NFT owner as it removes _tokenId from the _from account:
VotingEscrow.sol#L504-L515
_burn() allows _tokenId to approved or owner, but calls _removeTokenFrom() with msg.sender as _from:
VotingEscrow.sol#L517-L528
This way if _burn() is called by an approved account who isnt an owner, it will revert on _removeTokenFrom()s assert(idToOwner[_tokenId] == _from) check.
Now burn() is used by merge():
VotingEscrow.sol#L1084-L1097
And withdraw():
VotingEscrow.sol#L842-L864
Consider changing _removeTokenFrom() argument to be the owner:
VotingEscrow.sol#L517-L528
pooltypes (Velodrome) disputed
Alex the Entreprenerd (judge) increased severity to High and commented:
