Users approved for a single token id cannot withdraw or merge for that token id

Summary

Users approved for a single token id cannot withdraw or merge for that token id.

Vulnerability Detail

function withdraw(uint _tokenId) external nonreentrant {
        assert(_isApprovedOrOwner(msg.sender, _tokenId));
/.....
        _burn(_tokenId);
...../
    }

function merge(uint _from, uint _to) external {
/....
        _burn(_from);
...../
    }
   function _burn(uint _tokenId) internal {
        require(_isApprovedOrOwner(msg.sender, _tokenId), "caller is not owner nor approved");
        address tokenOwner = ownerOf(_tokenId);
        // Clear approval
        approve(address(0), _tokenId);
..../
function approve(address _approved, uint _tokenId) public {
        address tokenOwner = idToOwner[_tokenId];
        // Throws if `_tokenId` is not a valid NFT
        require(tokenOwner != address(0));
        // Throws if `_approved` is the current owner
        require(_approved != tokenOwner);
        // Check requirements
        bool senderIsOwner = (idToOwner[_tokenId] == msg.sender);
        bool senderIsApprovedForAll = (ownerToOperators[tokenOwner])[msg.sender];
@>        require(senderIsOwner || senderIsApprovedForAll);
        // Set the approval
        idToApprovals[_tokenId] = _approved;
        emit Approval(tokenOwner, _approved, _tokenId);
    }
    }

Both withdraw and merge check if the msg.sender is owner of the given token id or is approved to use it (either for all tokens from the same owner of specifically the one being used). They will also burn the token after carrying on their logic and clear its approvals.

Approvals are cleared using approve(address(0), _tokenId) which will fail if a msg.sender is only approved for that token id specifically.

The reason for the failure is senderIsOwner will not be true because since the call will be from any approvedSender and senderIsApprovedForAll will not be true since the operator is not approved for all the NFT's of the owner.

So this makes the require check for the OR fail

Impact

User approved for a single token cannot withdraw or merge. Users need to give permission for all their tokens to another user when they want that user to carry withdraw or merge operations for them.

Code Snippet

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/VotingEscrow.sol#L537 https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/VotingEscrow.sol#L543 https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/VotingEscrow.sol#L975 https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/VotingEscrow.sol#L1208

Tool used

Manual Review

Recommendation

Use the _clearApproval(owner, _tokenId) to clear the approvals in the _burn function.

Duplicate of #42

sherlock-audit / 2024-06-velocimeter-judging

MSaptarshi - Users approved for a single token id cannot withdraw or merge for that token id #569