0xAadi - `user.rewardSettled` become always zero due to the way `user.rewardDebt` is being calculated and deducted from `user.rewardSettled`.

[sherlock-admin3]

0xAadi

high

user.rewardSettled become always zero due to the way user.rewardDebt is being calculated and deducted from user.rewardSettled.

Summary

The SophonFarming.sol contract has an issue in both the _deposit() and withdraw() functions where the user.rewardSettled calculation can become inaccurate due to the way user.rewardDebt is being calculated. The current implementation deducts the total user.rewardDebt based on the entire user.amount, potentially leading to incorrect reward calculations.

Vulnerability Detail

In the _deposit() and withdraw() function, the current user.rewardSettled is calculate by user.amount and reducing the user.rewardDebt calculated previously. So the user.rewardSettled and user.rewardDebt are always same. This can lead to inaccuracies in reward calculations the user.rewardSettled become always zero.

Impact

The impact of this issue is that users may not receive the correct amount of rewards when they deposit funds to the contract.

Code Snippet

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

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

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

https://github.com/sherlock-audit/2024-05-sophon/blob/05059e53755f24ae9e3a3bb2996de15df0289a6c/farming-contracts/contracts/farm/SophonFarming.sol#L574C1-L595C29 https://github.com/sherlock-audit/2024-05-sophon/blob/05059e53755f24ae9e3a3bb2996de15df0289a6c/farming-contracts/contracts/farm/SophonFarming.sol#L699C1-L725C29

userAmount * pool.accPointsPerShare / 1e18 is used to calculate the users points and user.rewardDebt also uses the same calculation, so that user.rewardSettled always become zer0.

Tool used

Manual Review

Recommendation

Both the _deposit() and withdraw() functions should be updated to calculate the user.rewardDebt based on the actual amount being deposited or withdrawn rather than the whole user.amount. This will ensure that users receive the correct amount of rewards when they interact with the contract.

[sherlock-admin2]

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

0xmystery commented:

invalid because updatePool(_pid) is called prior to updating user.rewardSettled and eventually user.rewardDebt