sherlock-audit / 2024-08-flayer-judging

2 stars 0 forks source link

tvdung94 - Protocol will still use fee to cover exact-in swaps even when fee is not enough to fill #770

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

tvdung94

High

Protocol will still use fee to cover exact-in swaps even when fee is not enough to fill

Summary

Due to the logic error, beforeSwap() will still use fee to cover the swap ( for exact in swap) even when it is not enough to do so.

Vulnerability Detail

As commented in the code, in swaps using exact in option, using fee to cover the swaps should be avoided

               // If we cannot fulfill the full amount of the internal orderbook, then we want
                // to avoid using any of it, as implementing proper support for exact input swaps
                // is significantly difficult when we want to restrict them by the output token
                // we have available.

However, due to the logic error in the current implementation, fee will be used only when it is not enough to cover the swap (and vice versa), which is opposite with the intent stated in the comment.

            >>>    if (tokenOut <= uint(-params.amountSpecified)) { //@audit - opposite with the comments
                    // Update our hook delta to reduce the upcoming swap amount to show that we have
                    // already spent some of the ETH and received some of the underlying ERC20.
                    // Specified = exact input (ETH)
                    // Unspecified = token1
                    beforeSwapDelta_ = toBeforeSwapDelta(ethIn.toInt128(), -tokenOut.toInt128());
                } else {
                    ethIn = tokenOut = 0;
                }

Impact

Incorrect fee swap management (selling/buying token) might affect the price of the collection token negatively.

Code Snippet

https://github.com/sherlock-audit/2024-08-flayer/blob/main/flayer/src/contracts/implementation/UniswapImplementation.sol#L543-L556

Tool used

Manual Review

Recommendation

Consider this change:

  >>>    if (tokenOut >= uint(-params.amountSpecified)) {
                    // Update our hook delta to reduce the upcoming swap amount to show that we have
                    // already spent some of the ETH and received some of the underlying ERC20.
                    // Specified = exact input (ETH)
                    // Unspecified = token1
                    beforeSwapDelta_ = toBeforeSwapDelta(ethIn.toInt128(), -tokenOut.toInt128());
                } else {
                    ethIn = tokenOut = 0;
                }