sherlock-audit / 2024-06-magicsea-judging

2 stars 0 forks source link

0xc0ffEE - Can not cast vote after bribes registered #674

Closed sherlock-admin4 closed 1 month ago

sherlock-admin4 commented 2 months ago

0xc0ffEE

High

Can not cast vote after bribes registered

Summary

Whenever votes happen for that bribe's pool, votes are deposited for the given vote period and token id. However, the wrong check in deposit flow could revert the vote transaction

Vulnerability Detail

The function called by Voter contract. However, in this case, msg.sender (the Voter contract) can not be the owner of the tokenId, it should be the staking position owner

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

        emit Deposited(periodId, tokenId, _pool(), deltaAmount);
    }
...
    function _modify(uint256 periodId, uint256 tokenId, int256 deltaAmount, bool isPayOutReward)
        private
        returns (uint256 rewardAmount)
    {
        if (!IVoter(_caller).ownerOf(tokenId, msg.sender)) { // @audit-issue wrong check in deposit flow
            revert BribeRewarder__NotOwner();
        }

        // extra check so we dont calc rewards before starttime
        (uint256 startTime,) = IVoter(_caller).getPeriodStartEndtime(periodId);
        if (block.timestamp <= startTime) {
            _lastUpdateTimestamp = startTime;
        }
....

Impact

Can not integrate BribeRewarder contract with the Voter contract

Code Snippet

Tool used

Manual Review

Recommendation

Because the check above could be used in the claim flow, it is recommended that we should separate the checks for deposit flow and claim flow, i.e. move the check outside the function _modify

Duplicate of #39