dhank - MasterChefV2.sol :: User will get incorrect reward when someone in the pool has called emergencyWithdrawal()

[sherlock-admin2]

dhank

High

MasterChefV2.sol :: User will get incorrect reward when someone in the pool has called emergencyWithdrawal()

Summary

User will get incorrect reward when someone in the pool has called emergencyWithdrawal()

Vulnerability Detail

When a user wants to emergencyWithdraw from a pool , he calls emergencyWithdraw(uint256 pid). This will reduce the amount[userAccount] and the totalAmount.

But it is not updating the reward.accDebtPerShare and the reward.lastUpdateTimestamp which causes other providers to earn incorrect reward since reward.accDebtPerShare depends on the totalAmount.

    function emergencyWithdraw(uint256 pid) external override {
        //.. the rewards are lost from the lastUpdatedTime and in next time onwards it will calculate from scratch
        Farm storage farm = _farms[pid];

        uint256 balance = farm.amounts.getAmountOf(msg.sender);
        int256 deltaAmount = -balance.toInt256();

        farm.amounts.update(msg.sender, deltaAmount);
        //.. @audit farms reward needs to be updated using oldtotalSupply else other users will suffer reward loss. accRewardDebt is dependent on totalSupply.
        farm.token.safeTransfer(msg.sender, balance);

        emit PositionModified(pid, msg.sender, deltaAmount, 0);
    }

We are not updating the farm.rewards when the totalSupply is reduced.

Next time when a user claims his reward from that pool , getTotalRewards() is called initially

     function getTotalRewards(
       ....
    ) internal view returns (uint256) {
        if (totalSupply == 0) return 0;

        uint256 lastUpdateTimestamp = rewarder.lastUpdateTimestamp;
        uint256 timestamp = block.timestamp > endTimestamp ? endTimestamp : block.timestamp;

        return timestamp > lastUpdateTimestamp ? (timestamp - lastUpdateTimestamp) * rewardPerSecond : 0; 
    }

Here rewarder.lastUpdateTimestamp is from the timestamp before the user has withdrawn in energency, Hence totalDeposit value is used to calculate debtPershare form rewarder.lastUpdateTimestamp to current timestamp even tohugh totalDeposited is updated in the mid.

  function getDebtPerShare(uint256 totalDeposit, uint256 totalRewards) internal pure returns (uint256) {
        return totalDeposit == 0 ? 0 : (totalRewards << Constants.ACC_PRECISION_BITS) / totalDeposit;
    }

So when

Impact

User will get incorrect reward when someone in the pool has called emergencyWithdrawal()

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/MasterchefV2.sol#L326-L337

Tool used

Manual Review

Recommendation

Call reward.update() before ampount.update()

Duplicate of #376