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

6 stars 4 forks source link

Using `payable.transfer` functions in `WithdrawFacet.sol` and `Libasset.sol` is not usable for smart contract calls due to possible shortage of gas. #193

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/LibAsset.sol#L79-L85 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/WithdrawFacet.sol#L20-L38

Vulnerability details

Impact

Withdrawals and transferERC20 tokens are executed via transferERC20() and withdraw() functions. Since these functions calls with a fixed amount of gas, it's not yet guaranteed to reach to the destination if the receiver is a smart contract.

Proof of Concept

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibAsset.sol#L79-L85 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/WithdrawFacet.sol#L20-L38

Outlines are here : https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

Tools Used

Static testing

Recommended Mitigation Steps

Team can consider using call.value(amount)

H3xept commented 2 years ago

Fixed in lifinance/lifi-contracts@274a41b047b3863d9ae232eefea04896dc32d853

H3xept commented 2 years ago

Duplicate of #14