radin200 - Users are able to block bribing in pools for any voting period

[sherlock-admin2]

radin200

High

Users are able to block bribing in pools for any voting period

Summary

Users are able to block BribeRewarder::_bribe in pools for any voting period, by creating and register their own BribeRewarders to reach the limit with.

Vulnerability Detail

As we can see, nothing stops mistrustful user from creating their own BribeRewarder from RewarderFactory

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

 function _cloneBribe(RewarderType rewarderType, IERC20 token, address pool) 
     private 
     returns (IBribeRewarder rewarder) 
 { 
     if (rewarderType != RewarderType.BribeRewarder) revert RewarderFactory__InvalidRewarderType(); 
  
     IRewarder implementation = _implementations[rewarderType]; 
  
     if (address(implementation) == address(0)) revert RewarderFactory__ZeroAddress(); 
  
     IRewarder[] storage rewarders = _rewarders[rewarderType]; 
  
     bytes memory immutableData = abi.encodePacked(token, pool); 
     bytes32 salt = keccak256(abi.encodePacked(msg.sender, _nonces[msg.sender]++)); 
  
     rewarder = IBribeRewarder(ImmutableClone.cloneDeterministic(address(implementation), immutableData, salt)); 
  
     rewarders.push(rewarder); 
     _rewarderTypes[rewarder] = rewarderType; 
  
     rewarder.initialize(msg.sender); 
 } 

However, there is scenario where malicious actor could create 5 BribeRewareders(MAX_BRIBES_PER_POOL = 5), fund them and call bribe for as many periods as he want in the future with dustAmount inputed(funded amount should be >= totalAmount). Due to the Voter::onRegister call, actor's BribeRewarders will be recorded in Voter::_bribesPerPriod for the specified periods and pool address by reaching the maximum limit
https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/libraries/Constants.sol#L17

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);
        }
    }

Because of this actions and MAX_BRIBES_PER_POOL limit reached, protocol users won't be able to bribe voters for the blocked voting periods

Impact

1. Core Invariant broke, protocol users could be blocked from bribe rewards into voting pools
2. Voters' rewards would be 0 or dust amounts

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/rewarders/RewarderFactory.sol#L109-L113
https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/rewarders/RewarderFactory.sol#L152-L173

Tool used

Manual Review

Recommendation

Consider making that only trusted users are able create and register new BribeRewarders or increase MAX_BRIBES_PER_POOL to make blocking periods unworthy for attackers

Duplicate of #190