sherlock-audit / 2024-06-magicsea-judging

8 stars 5 forks source link

Icy Basil Seal - Loss of rewards in case MlumStaking is underfunded #703

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

Icy Basil Seal

Low/Info

Loss of rewards in case MlumStaking is underfunded

Summary

In case the MlumStaking contract lacks reward funds, and a user withdraws his staked tokens, he will not receive his intended reward amount, but his position will get destroyed.

This will not allow him to claim these rewards, even in case the contract gets funded again.

Vulnerability Detail

The _safeRewardTransfer() rewards function is designed to account for the case where there are less reward tokens in the contract then what should be paid out.

function _safeRewardTransfer(address _to, uint256 _amount) internal {
        uint256 rewardBalance = rewardToken.balanceOf(address(this));

        if (_amount > rewardBalance) {
            _lastRewardBalance = _lastRewardBalance - rewardBalance;
            rewardToken.safeTransfer(_to, rewardBalance);
        } else {
            _lastRewardBalance = _lastRewardBalance - _amount;
            rewardToken.safeTransfer(_to, _amount);
        }
    }

In case the user is entitled to more rewards, the contract will pay the balance, but will not account for the rewards not payed out.

function _harvestPosition(uint256 tokenId, address to) internal {
        .....
        // transfer rewards
        if (pending > 0) {
            // send rewards
            _safeRewardTransfer(to, pending); < no return value check 
        }
        emit HarvestPosition(tokenId, to, pending);
    }

In case the user withdraws the full position, he loses his rewards.

Impact

In case the MlumStaking contract is underfunded, user will lose his rewards in case the withdraws or harvests tokens.

Because rewards are calculated using balanceOf and updatePool is called on every harvest operation, the loss at most can be rounding error due to calculation. This is such a minimal impact that we decided to treat this issue as low.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L674-L686 https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L739-L749

Tool used

Manual Review

Recommendation

When withdrawing the whole balance of the contract, check if rewards are left and allow user to withdraw these.