0xAadi - Use of wrong parameter type in `Voter._checkRegisterCaller()` cause DoS on `Voter.onRegister()`

[sherlock-admin3]

0xAadi

Medium

Use of wrong parameter type in Voter._checkRegisterCaller() cause DoS on Voter.onRegister()

Summary

Voter._checkRegisterCaller() function using a wrong parameter type IBribeRewarder instead of IRewarder cuase Voter.onRegister() to fail

Vulnerability Detail

The vulnerability lies in the _checkRegisterCaller function from Voter contract.

The getRewarderType function defined in RewarderFactory only accept IRewarder as the parameter.

    function getRewarderType(IRewarder rewarder) external view returns (RewarderType) {
        return _rewarderTypes[rewarder];
    }

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/RewarderFactory.sol#L84C4-L86C6

But the _checkRegisterCaller function from Voter contract passing IBribeRewarder as the parameter.

    function _checkRegisterCaller(IBribeRewarder rewarder) internal view {
@>>    if (_rewarderFactory.getRewarderType(rewarder) != IRewarderFactory.RewarderType.BribeRewarder) {
            revert Voter__InvalidRegisterCaller();
        }
    }

This will cause the Voter.onRegister() which internally calls the _checkRegisterCaller will always revert.

Impact

DoS on Voter.onRegister() cause restricting bribe rewarder from registers itself.

Code Snippet

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

Tool used

Manual Review

Recommendation

Please update the code like the below one

    function onRegister() external override {
-       IBribeRewarder rewarder = IBribeRewarder(msg.sender);
+      IRewarder rewarder = IRewarder(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);
        }
    }

-  function _checkRegisterCaller(IBribeRewarder rewarder) internal view {
+  function _checkRegisterCaller(IRewarder rewarder) internal view {
        if (_rewarderFactory.getRewarderType(rewarder) != IRewarderFactory.RewarderType.BribeRewarder) { 
            revert Voter__InvalidRegisterCaller();
        }
    }

[0xSmartContract]

Considering the Solidity documentation and ABI coding, it appears that the two interfaces (IBribeRewarder and IRewarder) have the same address type and are therefore compatible during the calls. This actually makes it possible to pass the IBribeRewarder parameter as IRewarder and not cause any problems with the function's operation. False