sherlock-audit / 2024-05-sophon-judging

7 stars 6 forks source link

aslanbek - `_pendingPoints` returns more points than the user will actually receive #34

Closed sherlock-admin3 closed 5 months ago

sherlock-admin3 commented 5 months ago

aslanbek

medium

_pendingPoints returns more points than the user will actually receive

Summary

Internal function _pendingPoints is used by multiple view functions to retrieve the amount of rewards the user received for staking. There points are supposed to represent amount of SOPH each user will receive.

However, _pendingPoints uses a slightly different formula for computing the rewards in comparison to state-changing functions, and will return a value slightly different than the "real" one.

Vulnerability Detail

_pendingPoints adds accPointsPerShare * 18 and pointReward * 1e18 / lpSupply, and then divides the result by 1e36.

other functions use updatePool, which adds accPointsPerShare and pointReward / lpSupply, and then divide the result by 1e18.

Therefore, the former will have higher precision and return a slightly higher value.

Impact

View functions that return users' pending points will return a value higher than the actual one. As it is the only function that allows to retrieve users' accumulated rewards, and is going to be used for determining amount of SOPH to mint, it will result in slightly too many SOPH being minted.

Code Snippet

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

Tool used

Manual Review

Recommendation

-       uint256 accPointsPerShare = pool.accPointsPerShare * 1e18;
+       uint256 accPointsPerShare = pool.accPointsPerShare;
        {
        /*...*/
-           accPointsPerShare = pointReward *
-               1e18 /
+           accPointsPerShare = pointReward /
                lpSupply +
                accPointsPerShare;
        }

        return user.amount *
            accPointsPerShare /
-           1e36 +
+           1e18 +
            user.rewardSettled -
            user.rewardDebt;
    }-
sherlock-admin2 commented 5 months 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