sherlock-audit / 2024-07-sense-points-marketplace-judging

8 stars 5 forks source link

KupiaSec - The calculation of `fee` in `PointTokenVault.redeemRewards` function is unfair #176

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

KupiaSec

Medium

The calculation of fee in PointTokenVault.redeemRewards function is unfair

Summary

In PointTokenVault.redeemRewards function, the fee amount is calculated using pTokensToBurn rounded up instead of amountToClaim. This amount is unfair for users and it leads the users' loss of funds.

Vulnerability Detail

In the redeemRewards function, pTokensToBurn is calculated using round up operation with rewardsPerPToken value. The fee amount to pay is calculated using round up operation with redeemableWithFee value derived from pTokensToBurn. As a result, users should pay more fee by the rounding operation.

Let's consider the following scenario:

L191:        uint256 pTokensToBurn = FixedPointMathLib.divWadUp(amountToClaim, rewardsPerPToken);

fee = 2 * 1e28 / 1e18 * 0.05e18 / 1e18 = 1e9 from L211.

L209:           uint256 redeemableWithFee = pTokensToBurn - feelesslyRedeemable;
            // fee = amount of pTokens that are not feeless * rewardsPerPToken * redemptionFee
L211:           fee = FixedPointMathLib.mulWadUp(
                    FixedPointMathLib.mulWadUp(redeemableWithFee, rewardsPerPToken), redemptionFee
                );

As a result, Alice should pay 1e9 amount of reward token to claim 1e10 + 1 amount instead of (1e10 + 1) 0.05 = 0.5e9. Alice also should pay same amount of reward token to claim (2e10 - 1) amount(`pTokensToBurn = ((2e10 - 1) 1e18) / 1e28 rounded up = 2from L191). This is unfair for users to pay fee. The higher therewardsPerPToken`, the greater the unfairness.

Impact

Unfair calculation of fee amount causes the user's loss of funds.

Code Snippet

https://github.com/sherlock-audit/2024-07-sense-points-marketplace/blob/076bf833f4dc1418e93c8172e4a4110344f1c812/point-tokenization-vault/contracts/PointTokenVault.sol#L211

Tool used

Manual Review

Recommendation

It is recommended to calculate the fee from amountToClaim instead of redeemableWithFee:

            fee = FixedPointMathLib.mulWadUp(
-               FixedPointMathLib.mulWadUp(redeemableWithFee, rewardsPerPToken), redemptionFee
+               amountToClaim * redeemableWithFee / pTokensToBurn, redemptionFee
            );