0xblack_bird - `Withdraw` lacks proper checks ,resulting in unexpected state

[sherlock-admin3]

0xblack_bird

medium

Withdraw lacks proper checks ,resulting in unexpected state

Summary

withdraw function doesnt check if Boost amount can be greater than deposit amount during withdrawal,resulting in system being in an unexpected state.

Vulnerability Detail

In withdrawal function

function withdraw(uint256 _pid, uint256 _withdrawAmount) external {
        if (isWithdrawPeriodEnded()) {
            revert WithdrawNotAllowed();
        }
        if (_withdrawAmount == 0) {
            revert WithdrawIsZero();
        }

        PoolInfo storage pool = poolInfo[_pid];
        UserInfo storage user = userInfo[_pid][msg.sender];
        updatePool(_pid);

        uint256 userDepositAmount = user.depositAmount;

        if (_withdrawAmount == type(uint256).max) {
            _withdrawAmount = userDepositAmount;
        } else if (_withdrawAmount > userDepositAmount) {
            revert WithdrawTooHigh(userDepositAmount);
        }

        uint256 userAmount = user.amount;
        user.rewardSettled =
            userAmount *
            pool.accPointsPerShare /
            1e18 +
            user.rewardSettled -
            user.rewardDebt;

        user.depositAmount = userDepositAmount - _withdrawAmount;
        pool.depositAmount = pool.depositAmount - _withdrawAmount;

        userAmount = userAmount - _withdrawAmount;

        user.amount = userAmount;
        pool.amount = pool.amount - _withdrawAmount;

        pool.lpToken.safeTransfer(msg.sender, _withdrawAmount);

        user.rewardDebt = userAmount *
            pool.accPointsPerShare /
            1e18;

        emit Withdraw(msg.sender, _pid, _withdrawAmount);
    }

There are no explicit checks mentioned here checking whether boostAmount can be greater than depositAmount.The vulnerability lies in the fact the deposit ensures a check whether boostAmount can never be greater than deposit amount.

  function _deposit(uint256 _pid, uint256 _depositAmount, uint256 _boostAmount) internal {
        if (isFarmingEnded()) {
            revert FarmingIsEnded();
        }
        if (_depositAmount == 0) {
            revert InvalidDeposit();
        }
        if (_boostAmount > _depositAmount) {
            revert BoostTooHigh(_depositAmount);
        }

However such a check is absent in the withdraw function which can cause an invariant break.

Impact

would drive system to a state where the boostamount would be greater than deposit amount causing users fund loss.

Code Snippet

https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L699C3-L743C1

Tool used

Manual Review

Recommendation

ensure proper checks in the withdraw to ensure deposit should be always greater than boostAmount

[sherlock-admin4]

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

invalid because of incorrect assumptions. The withdrawl logic is working as intended