Users can withdraw more than deposited balance

Summary

Due to the int256 type used in the _modify function, users can have negative balance on MasterChef contract

Vulnerability Detail

On withdraw as seen here https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L306

and https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L539

We see the _modify function takes the int256 parameter as delta amount, there is no check to see for balance and if balance + delataAmount > 0 like below uint256 balance = farm.amounts.getAmountOf(msg.sender);

Impact

High impact, as contract can be drained by attackers

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L539-L563

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L306

function withdraw(uint256 pid, uint256 amount) external override {
        _modify(pid, msg.sender, -amount.toInt256(), true);

        if (amount > 0) _farms[pid].token.safeTransfer(msg.sender, amount);
    }

Tool used

Manual Review

Recommendation

function withdraw(uint256 pid, uint256 amount) external override {
        _modify(pid, msg.sender, -amount.toInt256(), true);
      uint256 balance = farm.amounts.getAmountOf(msg.sender);
      require(balance <= amount, "amount > balance");
        if (amount > 0) _farms[pid].token.safeTransfer(msg.sender, amount);
    }

Duplicate of #545

sherlock-audit / 2024-06-magicsea-judging

John_Femi - Users can withdraw more than deposited balance #682