31071 - [SC - Critical] User can steal bribes and prevent other users f...

Submitted on May 12th 2024 at 08:42:33 UTC by @imsrybr0 for Boost | Alchemix

Report ID: #31071

Report type: Smart Contract

Report severity: Critical

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

Impacts:

• Theft of unclaimed yield

Description

Brief/Intro

User can steal bribes and prevent other users from claiming theirs.

Vulnerability Details

// ...
contract Voter is IVoter {
	// ...
    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]); // <==== Audit
        }
    }

    function distribute() external {
        uint256 start = 0;
        uint256 finish = pools.length;

        for (uint256 x = start; x < finish; x++) {
            // We don't revert if gauge is not alive since pools.length is not reduced
            if (isAlive[gauges[pools[x]]]) {
                _distribute(gauges[pools[x]]); // <==== Audit
            }
        }

        IMinter(minter).updatePeriod();
    }

    function _distribute(address _gauge) internal {
        // Distribute once after epoch has ended
        require(
            block.timestamp >= IMinter(minter).activePeriod() + IMinter(minter).DURATION(),
            "can only distribute after period end"
        );

        uint256 _claimable = claimable[_gauge];

        // Reset claimable amount
        claimable[_gauge] = 0;

        _updateFor(_gauge);

        if (_claimable > 0) {
            IBaseGauge(_gauge).notifyRewardAmount(_claimable);
        }

        IBribe(bribes[_gauge]).resetVoting(); // <==== Audit

        emit DistributeReward(msg.sender, _gauge, _claimable);
    }
	// ...
}

// ...
contract Bribe is IBribe {
	// ...
	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]); // <==== Audit

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

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

	function resetVoting() external {
        require(msg.sender == voter);
        totalVoting = 0; // <==== Audit
    }
	// ...
}

Voter@distribute resets the total votes to 0 on the given Gauge corresponding Bribe and doesn't trigger a voting checkpoint. It also doesn't change the individual user balances and total supply causing Bribe and Voter to be out of sync until all previous voter vote again.

Additionally, Voter@claimBribes triggers a checkpoint for the given token id.

Combined, they allow for situation where a user :

• Votes in Epoch N

• In Epoch N + 1 :

  • Does not vote.

  • Calls Voter@claimBribes to claim any bribes from Epoch N, triggering a checkpoint and activating rewards for this epoch (.i.e Epoch N + 1, claimable in Epoch N + 2).

  • Calls Voter@distribute to reset total votes. Rewards will now be calculated based on new voters, but still distributed to everyone with a checkpoint.

• User can keep claiming a share of bribes in future epochs at the expense of other voters.

Impact Details

The impact of this issue will vary based on the participating voters voting power and the pool allocations and can span from rewards being fully stolen to partially stolen and preventing all / some users from claiming theirs because of a lack of funds to cover their shares.

References

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

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

Proof of Concept

    function testStealBribes() public {
        uint256 tokenId1 = createVeAlcx(admin, 2 * TOKEN_1, MAXTIME, false);