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.
gkrastenov
High
Incorrect check of ownerOf for tokenId during voting
Summary
Incorrect check of
ownerOf
fortokenId
during voting.Vulnerability Detail
After the votes are cast, users get rewarded as the
_notifyBribes
function calls thedeposit
function of theBribeRewarder
contract, if the rbribe eward contract exists for the current period and pool. Thedeposit
function first checks if the caller is theVoter
contract through theonlyVoter
modifier, which is true.After that, in the
_modify
function, it is checked if themsg.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 theVoter
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.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
Tool used
Manual Review
Recommendation
The original caller of the transaction should be checked in the
_modify
function to see if they own the giventokenId
.Duplicate of #39