sherlock-audit / 2024-06-velocimeter-judging

11 stars 7 forks source link

hulkvision - Users can call `reset` on their token even if they don't have active votes, griefing potential token buyer/receiver #689

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 4 months ago

hulkvision

Medium

Users can call reset on their token even if they don't have active votes, griefing potential token buyer/receiver

Summary

Users can vote on certain gauges in the Voter contract, allowing them to "withdraw" their votes using the reset function, a note on this function, is that it sets lastVoted for that token ID. However, that function doesn't check if the token has active votes before allowing the user to reset them. This introduces an issue, especially when users move/transfer/sell their tokens, more below.

Vulnerability Detail

A bit of context, in VotingEscrow::_transferFrom, there's a check to block the transferring of tokens in case that token has active votes.

require(attachments[_tokenId] == 0 && !voted[_tokenId], "attached");

This is not there to block users from voting multiple times using the same token, as the vote is saved per token and not per owner. So, this is there to allow a token receiver to immediately have the ability to use that token and vote on gauges.

So a user can use the missing check anomaly to grief NFT buyers (remember tokens are NFTs and are tradable), so a user puts his token for sale, and just before the transferal that user front runs the transfer TX and calls Voter::reset, this blocks the buyer from voting on any gauge in the current epoch.

Impact

Manual Review

Recommendation

Add the following in Voter::reset:

require(poolVote[_tokenId].length > 0, "No active votes");
nevillehuang commented 3 months ago

Invalid, user still can vote, lastVoted is not updated when _reset is called