search

code-423n4 / 2022-03-lifinance-findings

6 stars 4 forks source link

User will lose value on swap-and-bridge / multi-swap #181

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L30

Vulnerability details

Impact

In a swap-and-bridge or multi-swap, user have to supply exact amount of input token for each step. Any positive slippage will be captured by the contract and any negative slippage will cause the swap to revert. This is a sub-optimial behavior as it will make user to lose value or have to risk wasting gas on revert. It is not advisable for LiFi to have positive slippage as a revenue source as it provide very poor UX.

Proof of Concept

https://etherscan.io/tx/0xe78c36dd2c2f21cade00a4099701b9c9f82acc8da568e1048a4d7287ce2e45b0 In the above tx, the user tried to swap 0.3 ETH -> 2,650 LYRA -> cbridge Since the 1inch swap returned 2699 LYRA, the positive slippage of 44 LYRA is captured by the contract

Recommended Mitigation Steps

Add an option that use max amount available as input amount

H3xept commented 2 years ago

Duplicate of #66

We are aware that the contract allows users to use latent funds, although we disagree on it being an issue as no funds (ERC20 or native) should ever lay in the contract. To make sure that no value is ever kept by the diamond, we now provide refunds for outstanding user value (after bridges/swaps).

gzeoneth commented 2 years ago

Submitted by contest judge.