gkrastenov - Incorrect check of ownerOf for tokenId during voting

[sherlock-admin4]

gkrastenov

High

Incorrect check of ownerOf for tokenId during voting

Summary

Incorrect check of ownerOf for tokenId during voting.

Vulnerability Detail

After the votes are cast, users get rewarded as the _notifyBribes function calls the deposit function of the BribeRewarder contract, if the rbribe eward contract exists for the current period and pool. The deposit function first checks if the caller is the Voter contract through the onlyVoter modifier, which is true.

function deposit(uint256 periodId, uint256 tokenId, uint256 deltaAmount) public onlyVoter {
        _modify(periodId, tokenId, deltaAmount.toInt256(), false);

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

After that, in the _modify function, it is checked if the msg.sender (Voter contract) is the owner of the token, which is false because when the original owner of the token votes, he does not transfer his token to the Voter contract. This requirement will fail and the whole transaction will revert, blocking voting for the current period and pool and preventing the owner of the token of the extra bribe reward.

    function _modify(uint256 periodId, uint256 tokenId, int256 deltaAmount, bool isPayOutReward)
        private
        returns (uint256 rewardAmount)
    {   

        //@audit-issue H1: msg.sender is Voter contract, which is not owner
        // of tokenId and this check will always fail
        if (!IVoter(_caller).ownerOf(tokenId, msg.sender)) {
            revert BribeRewarder__NotOwner();
        }

        // ... 
    }

Impact

Blocking the voting and receiving the extra bribe reward.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

The original caller of the transaction should be checked in the _modify function to see if they own the given tokenId.

Duplicate of #39