sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

Ace-30 - swapperImpl.flash() does not account for swap fees and price discrepancies between the oracle and swapper. #55

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Ace-30

medium

swapperImpl.flash() does not account for swap fees and price discrepancies between the oracle and swapper.

Summary

In the swapperImpl contract, a trader must pay an additional amount for the swap fee. Therefore, the price the trader gets will always be higher than the oracle price. To successfully call the flash() function, the trader must keep an additional amount of tokenToBeneficiary (beyond the amountToBeneficiary) in their own address to account for this price difference.

Vulnerability Detail

The swapperImpl contract allows traders to call the flash() function to withdraw tokens in return for sending tokenToBeneficiary to the beneficiary. However, traders have to pay a swap fee (0.3% on Uniswap) so the price they get is lower than the Oracle price. This means the amount of tokens the trader receives is less than the amount calculated by the flash() function. Therefore, traders have no incentive to call flash() unless they own the diversifier. For a successful flash transaction, the owner of the diversifier should keep an extra amount (0.3%) of tokenToBeneficiay.

Impact

flash() is not cost-efficient for traders and the owner should always keep tokenToBeneficiary to be able to flash to swapperImpl

Code Snippet

https://github.com/0xSplits/splits-swapper/blob/83ce1124767a097aac37d1cd162a9b27bbc48701/src/SwapperImpl.sol#L227-L255

Tool used

Manual Review

Recommendation

Increase the value of amountToBeneficiary to account for the swap fee and provide an incentive for third-party traders to invoke the flash() function. The increment can be a variable, adjustable by the owner, to guarantee it encompasses the swap fee and offers an appealing incentive for traders.