sherlock-audit / 2024-05-sophon-judging

1 stars 1 forks source link

0xAadi - Unnecessary Scaling in _pendingPoints() Function #228

Closed sherlock-admin2 closed 1 month ago

sherlock-admin2 commented 1 month ago

0xAadi

medium

Unnecessary Scaling in _pendingPoints() Function

Summary

The _pendingPoints() function in the SophonFarming.sol contract used to get the points which is used to airdrop to users later. But, the _pendingPoints() function uses a different scaling mechanism other than _deposit() used, which results in points being calculated differently from the _deposit() function.

Vulnerability Detail

The issue lies in the unnecessary scaling with in the _pendingPoints() function, which causes points to be calculated incorrectly compared to the _deposit() function.

Please see the code

357:    function _pendingPoints(uint256 _pid, address _user) internal view returns (uint256) {
358:        PoolInfo storage pool = poolInfo[_pid];
359:        UserInfo storage user = userInfo[_pid][_user];
360:
361: @-->   uint256 accPointsPerShare = pool.accPointsPerShare * 1e18;
362:
363:        uint256 lpSupply = pool.amount;
364:        if (getBlockNumber() > pool.lastRewardBlock && lpSupply != 0) {
365:            uint256 blockMultiplier = _getBlockMultiplier(pool.lastRewardBlock, getBlockNumber());
366:
367:            uint256 pointReward =
368:                blockMultiplier *
369:                pointsPerBlock *
370:                pool.allocPoint /
371:                totalAllocPoint;
372:
373: @-->       accPointsPerShare = pointReward *
374:                1e18 /
375:                lpSupply +
376:                accPointsPerShare;
377:        }
378:        
379:            return user.amount *
380:            accPointsPerShare /
381: @-->       1e36 +
382:            user.rewardSettled -
383:            user.rewardDebt;
384:    }

Please see line 361 and 373, which is used a different scaling than _deposit() and in line 381 scaled down with 1e36 to balance the previous scaling but it should scaled dow with 1e18 too, Please see the _deposit()

Impact

This issue can lead to discrepancies in the calculation of points, potentially affecting the accuracy of rewards earned by users.

Code Snippet

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

Tool used

Manual Review

Recommendation

Remove the unnecessary scaling with 1e18 in the _pendingPoints() function to ensure consistent calculation of points with the deposit() function.

sherlock-admin2 commented 1 month ago

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