`SwapRouter.sol` functions doesn't refund unspent ETH after swapping

Github username: @0xRizwan Twitter username: 0xRizwann Submission hash (on-chain): 0xae96b7a671931fc3260e21d6fb1413410a2f350138d0a89d3d21f12ced6528c2 Severity: high

Description: Description:

SwapRouter allows swapping of ETH for ERC20 tokens.

The difference between selling ETH and an ERC20 token is that the contract can compute and request from the user the exact amount of ERC20 tokens to sell, but, when selling ETH, the user has to send the entire amount when making the call (i.e. before the actual amount was computed in the contract).

As swaps made via SwapRouter can be partial, there are scenarios when ETH can be spent partially. However, the contract doesn't refund unspent ETH in such scenarios:

1) When sqrtPriceLimitX96 is set (SwapRouter.sol#L90, SwapRouter.sol#L190), the swap will be interrupted when the limit price is reached, and some ETH can be left unspent.

2) A swap can be interrupted earlier when there's not enough liquidity in a pool.

3) Positive slippage can result in more efficient swaps, causing exact output swaps to leave some ETH unspent (even when it was pre-computed precisely by the caller).

As a result, SwapRouter can hold some leftover ETH after a swap was made. There is no withdraw function to withdraw the left ETH from contract, Therefore the ETH will be stucked in contract which result in causing a loss to the SwapRouter user.

Recommendation:

Consider refunding unspent ETH for below functions.

1) SwapRouter.exactInputSingle(),

2) SwapRouter.exactInput(),

3) SwapRouter.exactInputSingleSupportingFeeOnTransferTokens(),

4) SwapRouter.exactOutputSingle(),

5) SwapRouter.exactOutput().

For example:


+   import './libraries/TransferHelper.sol';

+    function refundETH() internal {
+        if (address(this).balance > 0) +TransferHelper.safeTransferNative(msg.sender, address(this).balance);
+    }

    function exactInputSingle(
        ExactInputSingleParams calldata params
    ) external payable override checkDeadline(params.deadline) returns (uint256 amountOut) {
        amountOut = exactInputInternal(
            params.amountIn,
            params.recipient,
            params.limitSqrtPrice,
            SwapCallbackData({path: abi.encodePacked(params.tokenIn, params.tokenOut), payer: msg.sender})
        );
        require(amountOut >= params.amountOutMinimum, 'Too little received');
+    refundETH();
    }

References:

This issue is directly reference from this issue found in Velodrome finance audit at Spearbit.

For recommended mitigation changes references, please check this commit by Velodrome finance.

hats-finance / Fenix-Finance-0x83dbe5aa378f3ce160ed084daf85f621289fb92f

`SwapRouter.sol` functions doesn't refund unspent ETH after swapping #43