Adding pool with _withUpdate being false will lead to incorrect calcualation of point rewards

Summary

Adding pool with _withUpdate being false will lead to incorrect calcualation of point rewards

Vulnerability Detail

When adding a pool the owner is given the option to add the pool and set the parameter _withUpdate to false. This is a problem because if the owner where to add a new update with _withUpdate as false, this will cause other pool's reward calcuation to be off.

in the add fucntion with no update we only update the totalAllocPoint variable

        totalAllocPoint = totalAllocPoint + _allocPoint;

this is problematic because if a pool who has been earning rewards under a lower totalAllocPoint is not updated, before the addition of the new pool, their previous rewards will be incorrectly calulated.

Let us assume that a user of pool A which was added 10 blocks ago, deposits for 10 blocks and gains 10 point under the totalAllocPoint of 100 his reward for the week will be

            uint256 pointReward =
                blockMultiplier *
                pointsPerBlock *
                pool.allocPoint /
                totalAllocPoint;

assume block multiplier = 100 points perblock = 10 and alloc point = 10 totalAllocPoint = 200

his point reward will = 50

now if a new pool is added with 200 alloc points, the totalAllocPoint will be updated but his pool will not calculation showing above; 100 10 10 / 400 = 25 so now the user will have his reward = 25. even though he stakes for 10 block with previous totalAllocPoint, when his pool is updated, when he withdraws he will only be due 25 points instead of 50.

Impact

Users point calculation will be incorrect leading to loss of points for all users of a specific pool

Code Snippet

https://github.com/sherlock-audit/2024-05-sophon/blob/05059e53755f24ae9e3a3bb2996de15df0289a6c/farming-contracts/contracts/farm/SophonFarming.sol#L153-L187

Tool used

Manual Review

Recommendation

do not allow option of adding pool without update.

sherlock-admin4 commented 5 months ago

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

0xmystery commented:

invalid because it would require the admin to make a mistake by inputting a false boolean

sherlock-admin2 commented 5 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/sophon-org/farming-contracts/commit/6d117397d57a39a865cee0621cabf9154fb6fd71