Skip to content
This repository has been archived by the owner on Jan 12, 2025. It is now read-only.

blockchain555 - DoS by the Incorrect validation in function BribeRewarder.sol#_modify(). #259

Closed
sherlock-admin3 opened this issue Jul 15, 2024 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A High severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Jul 15, 2024

blockchain555

Medium

DoS by the Incorrect validation in function BribeRewarder.sol#_modify().

Summary

BribeRewarder.sol#deposit() is DoSed because of the incorrect validation in the BribeRewarder.sol#_modify() function.

Vulnerability Detail

The BribeRewarder.sol#deposit() is the function that Deposits votes for the given period and tokenId, only the voter contract can call this function by the onlyVoter() modifier.
The BribeRewarder.sol#deposit() is as follows.

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

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

As you can see above this code snippet, the _modify() function is called in the deposit() function, so here the caller (msg.sender) is the Voter contract.
However, the _modify() function checks whether the caller is the owner of tokenId.

    function _modify(uint256 periodId, uint256 tokenId, int256 deltaAmount, bool isPayOutReward)
        private
        returns (uint256 rewardAmount)
    {
264:    if (!IVoter(_caller).ownerOf(tokenId, msg.sender)) {
265:        revert BribeRewarder__NotOwner();
266:    }

        ...SNIP
    }

But the caller (msg.sender) cannot be the owner of tokenId (staking position), so it is reverted.

Impact

BribeRewarder.sol#deposit() is always reverted, so the vote() function of the Voter contract does not work correctly. As a result, the core function of the protocol is damaged.

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-L298

Tool used

Manual Review

Recommendation

The BribeRewarder.sol#_modify() function has to be modified as follows.

    function _modify(uint256 periodId, uint256 tokenId, int256 deltaAmount, bool isPayOutReward)
        private
        returns (uint256 rewardAmount)
    {
---     if (!IVoter(_caller).ownerOf(tokenId, msg.sender)) {
+++     if (!IVoter(_caller).ownerOf(tokenId, msg.sender) && isPayOutReward) {
            revert BribeRewarder__NotOwner();
        }

        ...SNIP
    }

Duplicate of #39

@github-actions github-actions bot added duplicate High A High severity issue. labels Jul 21, 2024
@sherlock-admin3 sherlock-admin3 added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Jul 22, 2024
@sherlock-admin4 sherlock-admin4 changed the title Abundant Pickle Rattlesnake - DoS by the Incorrect validation in function BribeRewarder.sol#_modify(). blockchain555 - DoS by the Incorrect validation in function BribeRewarder.sol#_modify(). Jul 29, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Jul 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A High severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants