Naresh - Users will not receive the correct reward tokens when MlumStaking is Under-supplied

[sherlock-admin3]

Naresh

Medium

Users will not receive the correct reward tokens when MlumStaking is Under-supplied

Summary

The _safeRewardTransfer() function of the MlumStaking contract. When the contract is undersupplied with reward tokens, it can cause the _harvestPosition() function to send fewer tokens than needed or revert subsequent activities. This issue arises because the missing tokens are not accounted for, potentially leading to users not receiving the expected rewards.

Vulnerability Detail

All calculations are rounded down since a lack of reward tokens in the contracts cannot be attributed to rounding errors. Therefore, the rounding function is redundant.

If the contract is undersupplied with reward tokens, the _harvestPosition() function will send fewer tokens than needed or revert all subsequent activities. This is particularly unsafe because the missing tokens are not accounted for. Consequently, a user who creates a staking position might invoke the _safeRewardTransfer() function and not receive the tokens they were supposed to.

   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);
        }
    }

Past Related Issues:

code-423n4/2022-02-concur-findings#244

code-423n4/2022-05-aura-findings#272

Impact

The functions addToPosition, _withdrawFromPosition,_lockPosition, harvestPosition, harvestPositionTo,harvestPositionsTo will not transfer the correct reward tokens when MlumStaking is undersupplied.

Code Snippet

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

Tool used

Manual Review

Recommendation

To ensure that the _safeRewardTransfer() function only executes when there is a positive reward balance, add the following require statement:

 require(rewardBalance > 0, "_safeRewardTransfer: balance is zero");

Alternatively, use the usual safeTransfer instead of _safeRewardTransfer.

[0xSmartContract]

The architectural preference of the MlumStaking contract is to send the available reward tokens in cases where there are insufficient reward tokens, as a better method than not sending any reward tokens at all. This preference is based on several important factors:

User Experience
It provides a more positive user experience for users to receive the available reward tokens rather than not receiving any rewards at all. This increases the satisfaction of users by receiving their rewards, even if only partially.

Better Than the Revert Alternative
In case of revert of the contract, all transactions of the users fail and the users do not receive any rewards. This means that the transactions are not completed and the users are disappointed. Incomplete reward sending ensures that the transactions are completed and this situation has a more positive result than reverting.

In addition; The probability of insufficient reward tokens is low. The project usually provides enough reward tokens and ensures that the system works properly. Therefore, the probability of this situation occurring is considered very low.

Incomplete Reward Sending and Monitoring
The lack of tracking of missing rewards can be seen as a weakness in the system. However, this is part of the architectural choice and was considered an acceptable risk for the operation of the project.