search

sujithsomraaj / lifi-swap-facet-v3-audit

1 Day Review - 5/28
0 stars 0 forks source link

Potential loss of overpaid native tokens due to unhandled excess payment #5

Open sujithsomraaj opened 6 months ago

sujithsomraaj commented 6 months ago

Context: GenericSwapFacetV3.sol#L142

Description: The contract contains multiple functions marked as payable, allowing users to send native tokens to the contract during those function calls.

However, if a user accidentally sends native tokens to these functions, they get locked up in the contract's balance. Hence, subsequent transactions could snatch the excessive native tokens to be returned to the user.

For example, consider the following function:

function swapTokensSingleV3ERC20ToERC20(
        bytes32 _transactionId,
        string calldata _integrator,
        string calldata _referrer,
        address payable _receiver,
        uint256 _minAmountOut,
        LibSwap.SwapData calldata _swapData
    ) external payable;

The above function is marked payable to save on gas, but whenever native tokens are sent, they get locked up in the contract.

Recommendation: Consider removing the payable keyword from the above-mentioned function and other related functions to avoid this scenario (or) block snatching by not returning address(this).balance in native swaps.

Impact: Likelihood: VERY LOW + Impact: HIGH = Severity: LOW

LI.FI:

Researcher:

0xDEnYO commented 6 months ago

I've removed the payable keyword from the ERC20To.... functions