Open sherlock-admin2 opened 2 months ago
The internal _modify()
call of the deposit()
function checks the identity of the token owner; this fails because msg.sender
is the voter contract, not the owner.So it is impossible to vote for pools that have bribe rewarders.
The protocol team fixed this issue in the following PRs/commits: https://github.com/metropolis-exchange/magicsea-staking/pull/13
The Lead Senior Watson signed off on the fix.
jennifer37
High
Non-functional vote() if there is one bribe rewarder for this pool
Summary
Permission check in BribeRewarder::deposit(), this will lead to vote() function cannot work if voted pool has any bribe rewarder.
Vulnerability Detail
When people vote for one pool, there may be some extra rewards provided by bribe rewarders. When users vote for one pool with some bribe rewarders, voter contract will call bribe rewarder's
deposit
function. However, in bribe rewarder'sdeposit()
function, there is one security check, the caller should be the NFT's owner, which is wrong. Because the voter contract call bribe rewarder'sdeposit()
, msg.sender is voter contract, not the owner of NFT. This will block all vote() transactions if this votes pool has any bribe rewarder.Poc
When alice tries to vote for one pool with one bribe rewarder, the transaction will be reverted with the reason 'BribeRewarder__NotOwner'
Impact
vote() will be blocked for pools which owns any bribe rewarders.
Code Snippet
https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L143-L147 https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L260-L269
Tool used
Manual Review
Recommendation
This security check should be valid in claim() function. We should remove this check from deposit().