Ryonen - BribeRewarder.deposit Reverts

[sherlock-admin3]

Ryonen

High

BribeRewarder.deposit Reverts

Summary

BribeRewarder._modify() is an internal function responsible for depositing/withdrawing, calculating rewards, and harvesting. In BribeRewarder, it is used twice: by the deposit function and the claim function. However, the deposit function always reverts.

Vulnerability Detail

The deposit function executes an onlyVoter modifier that does the following:

    function _checkVoter() internal view virtual {
        if (msg.sender != address(_caller)) {
            revert BribeRewarder__OnlyVoter();
        }
    }

This means that deposit can only be called by _caller.

On the other hand, within the _modify function, we have the following condition:

        if (!IVoter(_caller).ownerOf(tokenId, msg.sender)) {
            revert BribeRewarder__NotOwner();
        }

This requires that, to avoid reverting, the owner of the tokenId must be the contract at the _caller address, which does not make sense.

Impact

The deposit call can only revert.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L71-L74

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L393-L397

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

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L264-L266

Tool used

Manual Review

Recommendation

Properly check who the owner of the tokenId is by passing an additional parameter.

Duplicate of #39