ArsenLupin - Inflated amount of points will be received in the _pendingPoints

[sherlock-admin3]

ArsenLupin

high

Inflated amount of points will be received in the _pendingPoints

Summary

The _pendingPoints function, doesn't handle the decimals correctly, thus it results in the inflated amount of pending points returned

Vulnerability Detail

Last minutes of contest. Sorry, could PoC if necessary

Assume as an example:

1. pool.accPointsPerShare = 24999999999999999
2. blockMultiplier = 1e18 3.pointsPerBlock = 25e18
3. pool.allocPoint = 20000
4. totalAllocPoint = 60000
5. user.rewardSettled = 16666666666666666000
6. user.rewardDebt = 0

function _pendingPoints(uint256 _pid, address _user) internal view returns (uint256) {
        PoolInfo storage pool = poolInfo[_pid];
        UserInfo storage user = userInfo[_pid][_user];
        //pool.accPointsPerShare = pointReward / lpSupply + pool.accPointsPerShare is updated during the pool update!
        uint256 accPointsPerShare = pool.accPointsPerShare * 1e18;

        /**
        E: accPointsPerShare = 24999999999999999 * 1e18
            pointReward = 1e18 *  25e18 * 20000 / 60000
            accPointsPerShare1 = pointReward * 1e18 / 1000e18 + 24999999999999999000000000000000000
            return -> 1000e18 * accPointsPerShare1 / 1e36 + 16666666666666
            So, user expect to receive 33 points, which is bigger than pointsPerBlock? Is it correct?
         */
        uint256 lpSupply = pool.amount;
        if (getBlockNumber() > pool.lastRewardBlock && lpSupply != 0) {
            uint256 blockMultiplier = _getBlockMultiplier(pool.lastRewardBlock, getBlockNumber());

            uint256 pointReward = blockMultiplier * pointsPerBlock * pool.allocPoint / totalAllocPoint;
            //1e18 * 1e18 / lpSupply(which is in 1e18) + 5.  == 1e36 / 1e18 = 1e18
            accPointsPerShare = pointReward * 1e18 / lpSupply + accPointsPerShare;
        }
        //1e18 * 1e18 / 1e36 -> which could result in the rounding? Is it correct
        //500e18 * ((1e18 * 25e18 * 20000 / 60000) * 1e18 /  1000e18 + (24999999999999999 * 1e18)) 
        //500e18 * 16666666666666666166666666666666666666666666666666666666 / 1e36 + 16666666666666666000
        return user.amount * accPointsPerShare / 1e36 + user.rewardSettled - user.rewardDebt;
    }//(1e18 *  25e18 * 20000 / 60000) * 1e18 / 1000e18 + (24999999999999999 * 1e18)

Impact

Inflated amount of pending points will be returned

Code Snippet

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

Tool used

Manual Review

Recommendation

Adjust the decimals correctly. Simplify the logic

[sherlock-admin3]

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

0xmystery commented:

invalid because getPendingPoints() will not practically be used till point farming has ended. The higher precision adopted is by design