31141 - [SC - Critical] Permanent freezing of unclaimed yield of reward...

Permanent freezing of unclaimed yield of reward tokens in Bribe contract when attackers maliciously exploit voter.poke()

Submitted on May 13th 2024 at 10:11:55 UTC by @perseverance for Boost | Alchemix

Report ID: #31141

Report type: Smart Contract

Report severity: Critical

Target: https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/Bribe.sol

Impacts:

Permanent freezing of unclaimed yield
Permanent freezing of unclaimed royalties

Description

Brief/Intro

Bribe contracts allow bribing users with voting power to vote for a specific gauge. The contract allows bribed users to claim their bribes.

when the function notifyRewardAmount() is called, the reward token is sent from the msg.sender to bribe contract and kept in this contract as reward.

https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/Bribe.sol#L112

function notifyRewardAmount(address token, uint256 amount) external lock {

     IERC20(token).safeTransferFrom(msg.sender, address(this), amount);

    tokenRewardsPerEpoch[token][adjustedTstamp] = epochRewards + amount;
}

Holders of VeAlcx tokens after voting in Voter contract will earn some reward and can claim reward by calling function claimBribes in voter contract.

https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/Voter.sol#L332-L338

function claimBribes(address[] memory _bribes, address[][] memory _tokens, uint256 _tokenId) external {
        require(IVotingEscrow(veALCX).isApprovedOrOwner(msg.sender, _tokenId));

        for (uint256 i = 0; i < _bribes.length; i++) {
            IBribe(_bribes[i]).getRewardForOwner(_tokenId, _tokens[i]);
        }
    }

Reward is calculated as follow:

https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/Bribe.sol#L283C5-L300C6

function getRewardForOwner(uint256 tokenId, address[] memory tokens) external lock {
        require(msg.sender == voter, "not voter");
        address _owner = IVotingEscrow(veALCX).ownerOf(tokenId);
        uint256 length = tokens.length;
        for (uint256 i = 0; i < length; i++) {
            uint256 _reward = earned(tokens[i], tokenId);

            require(_reward > 0, "no rewards to claim");

            lastEarn[tokens[i]][tokenId] = block.timestamp;

            _writeCheckpoint(tokenId, balanceOf[tokenId]);

            IERC20(tokens[i]).safeTransfer(_owner, _reward);

            emit ClaimRewards(_owner, tokens[i], _reward);
        }
    }

The earned() internal function is used to calculate the reward for a user.

https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/Bribe.sol#L265-L278


function earned(address token, uint256 tokenId) public view returns (uint256) {

        // Redacted for simplicity 
        Checkpoint memory cp = checkpoints[tokenId][_endIndex];
        uint256 _lastEpochStart = _bribeStart(cp.timestamp);
        uint256 _lastEpochEnd = _lastEpochStart + DURATION;
        uint256 _priorSupply = votingCheckpoints[getPriorVotingIndex(_lastEpochEnd)].votes;

        // Prevent divide by zero
        if (_priorSupply == 0) {
            _priorSupply = 1;
        }

        if (block.timestamp > _lastEpochEnd) {
            reward += (cp.balanceOf * tokenRewardsPerEpoch[token][_lastEpochStart]) / _priorSupply;
        }

        return reward;
}

The _priorSupply is taken from votingCheckpoints[].votes. This votes are updated whenever the deposit function into Bribe is called when user vote via Voter contract.

https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/Bribe.sol#L306C8-L316C6

    function deposit(uint256 amount, uint256 tokenId) external {
        require(msg.sender == voter);

        totalSupply += amount;
        balanceOf[tokenId] += amount;

        totalVoting += amount;

        _writeCheckpoint(tokenId, balanceOf[tokenId]);
        _writeSupplyCheckpoint();
        _writeVotingCheckpoint();

        emit Deposit(msg.sender, tokenId, amount);
    }

_writeVotingCheckpoint() is called.

https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/Bribe.sol#L362-L372

function _writeVotingCheckpoint() internal {
        uint256 _nCheckPoints = votingNumCheckpoints;
        uint256 _timestamp = block.timestamp;

        if (_nCheckPoints > 0 && votingCheckpoints[_nCheckPoints - 1].timestamp == _timestamp) {
            votingCheckpoints[_nCheckPoints - 1].votes = totalVoting;
        } else {
            votingCheckpoints[_nCheckPoints] = VotingCheckpoint(_timestamp, totalVoting);
            votingNumCheckpoints = _nCheckPoints + 1;
        }
    }

The vulnerability

Vulnerability Details

With that basic understanding, I will explain the Vulnerability now.

The vulnerability is that when user deposit() by calling vote() function via Voter contract, then the _writeVotingCheckpoint() is called. Then the votingCheckpoints[].votes is updated to be the totalVoting. In the function deposit, the totalVoting is increased. But in function withdraw() the totalVoting is not decreasing.

https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/Bribe.sol#L309