snapishere - "deposit" function does not work therefore vote wont work in a proper implementation.

[sherlock-admin2]

snapishere

High

"deposit" function does not work therefore vote wont work in a proper implementation.

Summary

"deposit" function in bribe rewarder does not work for anyone that will be voting, causing the voter to revert when someone calls it.

Vulnerability Detail

The issue with this is that the bribeRewarder contract is not properly getting the original account when the call is coming from an external contract and that when _modify is called from deposit it has the restriction of the caller being the voter meaning checks with the tokenId and message.sender will fail as we can presume the Voter contract isn't calling the function with their own staked positions.

Impact

People can't vote.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L211
voter calls _notifyBribes.

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L221-L225
_notifyBribes calls the deposit function for each bribeRewarder.

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L143-L144
deposit contains a modifier onlyVoter.

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

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L393-L395
_checkVoter requires the the person who called the function is the voter/_caller contract so therefore to proceed the user must be the voter contract.

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L260-L264
then the deposit calls the internal _modify function this requires the user is the owner of the tokenId with the ownerOf function.

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L402-L403
the ownerOf function calls the erc721 contract in the mlumStaking contract the ownerOf function only allows the owner of the tokenId and that can only be one person.

Tool used

Manual Review

Recommendation

passing through approved and owner from erc721 in mlumstaking function and allowing the voter approval to all tokenId's this could also present other security issues or alternatively tx.origin in the bribeRewarder.

Duplicate of #39