sherlock-audit / 2024-01-napier-judging

9 stars 6 forks source link

jah - wrong logic when calculating a fee #72

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

jah

high

wrong logic when calculating a fee

Summary

in NapirPool function like swapUnderlyingForPt and swapUnderlyingForExactBaseLpToken doesn't transfer the swap fee and protocol fee from the swapper

Vulnerability Detail

both swapUnderlyingForPt and swapUnderlyingForExactBaseLpToken are using the logic when swapping pt to underlying token which is subtracting the fees from underlying amount instead it should have added the fees with underlyingIn
Both function uses PoolMath.swapUnderlyingForExactBaseLpToken(); to calculate the amount and when doing that it uses -calculateSwap function // Subtract swap fee // underlyingWithFee = underlyingNoFee - fee int256 netUnderlyingToAccount18 = netUnderlying18 - underlyingFee18; // Charge protocol fee on swap fee // This underlying will be removed from the pool reserve int256 underlyingToProtocol18 = (underlyingFee18 * pool.protocolFeePercent.toInt256()) / FULL_PERCENTAGE; and it will return these back to the main function swapUnderlyingForPt and swapUnderlyingForExactBaseLpToken and if we try to see the swapUnderlyingForPt on line 416 it will try to save the returned values like underlyingIn, swapFee and protocolFee and on line 437 it will try to call the router but you can see that in the arguments that is only using underlytingin only and it is not adding the swapFee nor the protocolFee so it is only asking the router to only send the underlyingin amount without the fees but instead it should have added all the fees in this case the user is not paying for a fees

Impact

the proper amount of fee will not be taken from the user in which the liquidity provider and the protocol will not get there share

Code Snippet

https://github.com/sherlock-audit/2024-01-napier/blob/main/v1-pool/src/NapierPool.sol#L437

Tool used

Manual Review

Recommendation

when calling the router we should add the fees with the underlyingin

sherlock-admin commented 7 months ago

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

tsvetanovv commented:

Invalid. The fees are included in the swap

takarez commented:

valid: medium(6)