hats-finance / Fenix--0x9d7765a7ebd5b6322a30797a44a5428531970d3d

0 stars 1 forks source link

in `setVoteDelay::vote` Users opted to vote for Killed Gauages will be forced to voting Delays. #62

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: ililhunterlili Submission hash (on-chain): 0x2abf270b24997b728ee03c9597b032a82888702290ef262f27e2d1709e505856 Severity: medium

Description: Description\

first of all we have to take into considerations that VOTE_DELAY can be at MAX_VOTE_DELAY due to this check

    function setVoteDelay(uint256 _delay) external VoterAdmin {
        require(_delay != VOTE_DELAY, "already set");
        require(_delay <= MAX_VOTE_DELAY, "max delay"); <<@
        emit SetVoteDelay(VOTE_DELAY, _delay);
        VOTE_DELAY = _delay;
    }

taking into considerations that VOTE_DELAY may be actually 1 Week then we see that at

    function vote(uint256 _tokenId, address[] calldata _poolVote, uint256[] calldata _weights) external nonReentrant {
        _voteDelay(_tokenId);
        require(IVotingEscrow(_ve).isApprovedOrOwner(msg.sender, _tokenId), "!approved/Owner");
        require(_poolVote.length == _weights.length, "Pool/Weights length !=");
        _vote(_tokenId, _poolVote, _weights);
        lastVoted[_tokenId] = _epochTimestamp() + 1; <<@
    }

we set lastVoted to the start of the current epoch

the problem rises from the fact that users unintentially may vote for killed Gauges without knowing that they are killed and in _vote there is no check for reverts for current scenario

    function _vote(uint256 _tokenId, address[] memory _poolVote, uint256[] memory _weights) internal {
......    SKIP

        for (uint i = 0; i < _poolCnt; i++) {
            if (isAlive[gauges[_poolVote[i]]]) _totalVoteWeight += _weights[i];
        }

        for (uint256 i = 0; i < _poolCnt; i++) {
            address _pool = _poolVote[i];
            address _gauge = gauges[_pool];

            if (isGauge[_gauge] && isAlive[_gauge]) {
......    SKIP
                require(votes[_tokenId][_pool] == 0);
                require(_poolWeight != 0);
......    SKIP
            }
        }
        if (_usedWeight > 0) IVotingEscrow(_ve).voting(_tokenId);
        totalWeightsPerEpoch[_time] += _totalWeight;
    }

as we see checks and potential reverts are inside the if (isGauge[_gauge] && isAlive[_gauge]) of the loop and this if statement won't come to be true

then this will lead to txn bein executed with no revert and the lastVoted[_tokenId] = _epochTimestamp() + 1; would be set in vote with no problems

now during the _voteDelay check, this will always revert and will lead to user losing the rewards of the current epoch

Attack Scenario\

  1. User see see good Gauges and vote for them in epochs and get good rewards in epoch 1
  2. Governance kills the Gauges
  3. User's voting balance increase and he wants to increse his votes in the gauag -> votes for the killed Gauges in epoch 2
  4. Txn executes and doesn't revert
  5. User will get no rewards for the epoch 2 if VOTE_DELAY == MAX_VOTE_DELAY or MAX_VOTE_DELAY - 1

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)