sherlock-audit / 2024-06-magicsea-judging

8 stars 5 forks source link

blockchain555 - A malicious attacker can damage the bribe distribution function at low cost. #641

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

blockchain555

Medium

A malicious attacker can damage the bribe distribution function at low cost.

Summary

Anyone can create a bribe reward and thus the attacker can control the distribution of the bribe.

Vulnerability Detail

As you know, in RewarderFactory.sol anyone can create a bribe reward.

    function createBribeRewarder(IERC20 token, address pool) external returns (IBribeRewarder rewarder) {
        rewarder = IBribeRewarder(_cloneBribe(RewarderType.BribeRewarder, token, pool));

        emit BribeRewarderCreated(RewarderType.BribeRewarder, token, pool, rewarder);
    }

BribeRewarder.sol uses fundAndBribe() and bribe() to distribute bribes to certain pools in a given period.

    function _bribe(uint256 startId, uint256 lastId, uint256 amountPerPeriod) internal {
        _checkAlreadyInitialized();
        if (lastId < startId) revert BribeRewarder__WrongEndId();
        if (amountPerPeriod == 0) revert BribeRewarder__ZeroReward();

        IVoter voter = IVoter(_caller);

        if (startId <= voter.getCurrentVotingPeriod()) {
            revert BribeRewarder__WrongStartId();
        }

        uint256 totalAmount = _calcTotalAmount(startId, lastId, amountPerPeriod);

        uint256 balance = _balanceOfThis(_token());

        if (balance < totalAmount) revert BribeRewarder__InsufficientFunds();

        _startVotingPeriod = startId;
        _lastVotingPeriod = lastId;
        _amountPerPeriod = amountPerPeriod;

        // create rewads per period
        uint256 bribeEpochs = _calcPeriods(startId, lastId);
        for (uint256 i = 0; i <= bribeEpochs; ++i) {
            _rewards.push();
        }

        _lastUpdateTimestamp = block.timestamp;

        IVoter(_caller).onRegister();

        emit BribeInit(startId, lastId, amountPerPeriod);
    }

But here this funtion calls onRegister() of Voter.sol.

    function onRegister() external override {
        IBribeRewarder rewarder = IBribeRewarder(msg.sender);

        _checkRegisterCaller(rewarder);

        uint256 currentPeriodId = _currentVotingPeriodId;
        (address pool, uint256[] memory periods) = rewarder.getBribePeriods();
        for (uint256 i = 0; i < periods.length; ++i) {
            // TODO check if rewarder token + pool  is already registered

            require(periods[i] >= currentPeriodId, "wrong period");
 @          require(_bribesPerPriod[periods[i]][pool].length + 1 <= Constants.MAX_BRIBES_PER_POOL, "too much bribes");
            _bribesPerPriod[periods[i]][pool].push(rewarder);
        }
    }

As you can see, the number of bribeRewarders in the pool that can distribute bribes in a given period cannot exceed MAX_BRIBES_PER_POOL (5). Therefore attacker only needs to create a BribeRewarder and consume the gas to run fundAndBribe().

Impact

The attacker manipulates the bribe distribution function to damage the core functionality of the protocol.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L111-L124 https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L226-L258

Tool used

Manual Review

Recommendation

Ensure that only permitted people call createBribeRewarder() in RewarderFactory.sol.

Duplicate of #190