blackhole - DoS Vulnerability in the deposit Function of BribeRewarder contract

[sherlock-admin3]

blackhole

High

DoS Vulnerability in the deposit Function of BribeRewarder contract

Summary

The deposit function in BribeRewarder always reverts due to a logical error.
This function is intended to be called by the voter contract when a user votes.
However, the _modify function, which is called within deposit, checks if the msg.sender is the owner of the tokenID.
Since the msg.sender is actually the voter contract and not the user, this check fails, causing the deposit function to revert and preventing the user from voting.

Vulnerability Detail

The deposit function includes the onlyVoter modifier, which ensures the msg.sender is the voter contract.
Inside the deposit function, the _modify function is called.
The _modify function checks if the msg.sender is the owner of the tokenID.
As the msg.sender is the voter contract (and not the actual user), this check fails, causing the deposit function to revert.

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

    function deposit(uint256 periodId, uint256 tokenId, uint256 deltaAmount) public onlyVoter {
        _modify(periodId, tokenId, deltaAmount.toInt256(), false); // @audit -high _caller is ownerOf(tokenId)?

        emit Deposited(periodId, tokenId, _pool(), deltaAmount);
    }

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

    function _modify(uint256 periodId, uint256 tokenId, int256 deltaAmount, bool isPayOutReward)
        private
        returns (uint256 rewardAmount)
    {
        if (!IVoter(_caller).ownerOf(tokenId, msg.sender)) {  // @audit msg.sender != owner of tokenId
            revert BribeRewarder__NotOwner();
        }
        ...
    }

Impact

Due to this issue, users are unable to vote, BribeRewarder contract cannot be used.

Code Snippet

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

Tool used

Manual Review

Recommendation

Update the _modify function to allow both the owner of the tokenID and the voter contract to use it.

    function _modify(uint256 periodId, uint256 tokenId, int256 deltaAmount, bool isPayOutReward)
        private
        returns (uint256 rewardAmount)
    {
-        if (!IVoter(_caller).ownerOf(tokenId, msg.sender)) {
+        if (!IVoter(_caller).ownerOf(tokenId, msg.sender) && msg.sender != _caller) {
            revert BribeRewarder__NotOwner();
        }

Duplicate of #39