search

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

6 stars 4 forks source link

Missing require can lead to funds lost #102

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/GenericSwapFacet.sol#L28-L30

Vulnerability details

Failed transfer with low level call won't revert

Impact

A missing require may cause user to lose funds if a corner case issue filed separately named Failed transfer with low level call won't revert. This may result in user funds lost because funds were transferred into this contract in preparation for the swap. The swap fails but doesn't revert. In GenericSwapFacet.sol there is no require to check that postSwapBalance is greater than 0. However, there is in the other facets.

Proof of Concept

  1. Alice uses Generic swap with 100 DAI
  2. Alice's 100 DAI are sent to the Swapper.sol contract
  3. The call on swap _swapData.callTo.call{ value: msg.value }(_swapData.callData); fails but returns success due to nonexisting contract
  4. postSwapBalance = 0 Other facets would revert here
  5. Alice receives nothing in return

Tools Used

Manual review

Recommended Mitigation Steps

add require(postSwapBalance > 0, "ERR_INVALID_AMOUNT");

H3xept commented 2 years ago

Fixed in lifinance/lifi-contracts@91870a578e8dd315b057acc5eb3370ffa0186208

H3xept commented 2 years ago

Duplicate of #76