search

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

6 stars 4 forks source link

Failed transfer with low level call won't revert #101

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Libraries/LibSwap.sol#L42-L46

Vulnerability details

Impact

swap is used throughout the code via _executeSwaps in Swapper.sol. According to Solidity Docs the call may return true even if it was a failure. 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. There is a way this can happen through GenericSwapFacet.sol due to a missing require that is present in the other facets which is a separate issue but gives this issue more relevance.

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
  5. Alice receives nothing in return

Tools Used

Manual review

Recommended Mitigation Steps

Check for contract existence

A similar issue was awarded a medium here

gzeoneth commented 2 years ago

Sponsor confirmed with fix.