Summary

The executeNativeSwapAndFee function is vulnerable to unchecked excess ETH, which may lead to unintended behavior and potential loss of funds. The function currently lacks proper validation and handling of excess ETH sent by the user, allowing it to be treated as part of the overall amount sent to the protocol's address here.

Vulnerability Details

If you want to exchange your ETH for a specific amount of tokens EUROs you can use the function. However, it's important to note that any extra ETH you spend won't be refunded automatically. So, make sure you're exchanging the exact amount you want. It's worth being mindful of this, as the executeNativeSwapAndFee function doesn't have any safeguards to prevent this from happening. One tip is to double-check your transaction details before executing the swap to avoid any mishaps.

So I tried to call uniswapRouter.refundETH() after a swap and I saw I received my leftover ETH back. The name(exactInputSingle) does not function as it looks.

Details on bug:

The bug is that SwapRouter doesn’t refund unspent ETH. If you sell ETH and set a limit price and it’s reached during the swap, then the input amount will be reduced, but you have already sent a bigger amount. The remaining amount will be left in the router.

The caller cannot know how much ETH will be spent by a swap: the Quoter contract, that’s used to calculate swaps before executing them, returns only the output amount, not the input one. Even if it had returned the input amount computed by a pool, the calculated input amount could have changed at the transaction execution time due to a price change, i.e. a slippage check would’ve been required on the input amount.

There’s another subtle detail: when checking that the caller has sent enough ETH, the contract uses the >= operation, which means it’s ok if the caller sends more ETH than needed for a swap. I guess this breaks one of the invariants, e.g. “SwapRouter must never take more user funds than required for a swap”. If the contract had used = instead, then all partial swaps would’ve failed. Which means refunding unspent ETH is the only solution to the problem.

Besides not refunding unspent ETH, SwapRouter allows anyone to withdraw ETH from the contract: anyone can withdraw the ETH SwapRouter hasn’t returned to you after the swap–it may be a MEV bot or simply anyone who calls refundETH after your transaction.

Poc

Alice decides to perform a token swap using the swap function provided by the smart contract.
Alice, with malicious intent or by mistake, decides to send a much larger amount of ETH than the calculated swap fee.
For example, if the intended swap fee is 100 wei, Alice sends 1000 wei here. The executeNativeSwapAndFee function is called with the calculated swap fee (100 wei) and the excessive ETH amount sent by Alice (900 wei).
The function attempts to send the calculated swap fee (100 wei) to the protocol's address which was the:

The excess ETH (900 wei) is not explicitly checked or refunded; it is treated as part of the overall amount sent to the protocol's address.
The excess ETH sent by Alice is not refunded, and it becomes part of the value used for the token swap execution.

This will lead to unintended behavior, loss of funds, or an incomplete swap if the excess ETH significantly exceeds the intended swap fee because the was no limit check I guess.

Impact

Users may inadvertently send more ETH than necessary, leading to an unintended loss of funds, as the excess ETH is not refunded.

Tools Used

Manual Review, GitHub

Recommendations

It would be great to have a system in place that rewards users for swapping native tokens. For instance, if a user decides to exchange their tokens for a relational token such as EUROs, and there is an excess amount of ETH sent in return, they should be able to retrieve the excess ETH deposited for the swap back. A more efficient way of achieving this would be by using the uniswap.refundETH() function and a fallback function(receive() payable{}) that would suit The Standard project.

Uniswap review on this vulnerability:

Uniswap says it works as expected. To not leave funds in SwapRouter, users should use the MultiCall functionality of the contract. MultiCall allows users to do multiple function calls in one transaction, thus Uniswap suggests that users should use MultiCall to swap tokens and call refundETH afterwards. They also said that the call to refundETH is made optional to reduce gas consumption for users.