sherlock-audit / 2024-06-velocimeter-judging

11 stars 7 forks source link

tvdung94 - nft max lock arrays could be filled with dust nfts #531

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

tvdung94

Medium

nft max lock arrays could be filled with dust nfts

Summary

When an nft is max lock enabled , it will be pushed into max_locked_nfts, an array that stores all nfts which have max lock enabled (from all users). Protocol will make sure that these locks duration will be updated by calling max_lock_bulk periodcally.

The problem is that users can just fill up this array with dust locks, making call. Since the sponsor mentioned that max_lock_bulk() is called by the protocol, array filled with dust locks might cause them either to waste a lot of gas or revert on max_lock_bulk() (when the required gas to loop through dust locks exceeds gas limit set by the protocol)

Vulnerability Detail

max_locked_nfts is an array storing all max lock enabled nfts. Users can enable an nft by calling enable_max_lock. Since there is no minimum amount required, users can enable locks with dust amount to fill up this array.

uint[] public max_locked_nfts;
function enable_max_lock(uint _tokenId) external {
        assert(_isApprovedOrOwner(msg.sender, _tokenId));
        require(maxLockIdToIndex[_tokenId] == 0,"enabled");

        max_locked_nfts.push(_tokenId);
        maxLockIdToIndex[_tokenId] = max_locked_nfts.length; 

        max_lock(_tokenId);
    }

Then whoever will call max_lock_bulk() (mostly the protocol), will either suffer gas griefing or potentially DOS (if gas required exceeds gas limit).

 function max_lock_bulk() public {
        max_lock_bulk(0,max_locked_nfts.length); 
    }

 function max_lock_bulk(uint start, uint finish) public {
        for (uint x = start; x < finish; x++) {
            max_lock(max_locked_nfts[x]);
        }
    }

Impact

Gas griefing or potentially DOS of max_lock_bulk function.

Code Snippet

https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/VotingEscrow.sol#L196 https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/VotingEscrow.sol#L883-L891 https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/VotingEscrow.sol#L927-L935

Tool used

Manual Review

Recommendation

Consider adding minimum amount requirement to be added into max lock array

nevillehuang commented 3 months ago

Invalid, not proven per required PoC by sherlock rules.