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

6 stars 4 forks source link

QA Report #107

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Report

Use a two-step process for ownership transfer

Transferring the owner in a two-step process is less error-prone than assigning the new owner directly.

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/OwnershipFacet.sol#L8

Here's an example from the Compound codebase: https://github.com/compound-finance/compound-protocol/blob/master/contracts/Timelock.sol#L45-L58

use .call() instead of .transfer() to send ETH

This makes the transfer prone to failure in case the receiving address is a contract: https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/WithdrawFacet.sol#L31

See SWC-134 for more infos.

H3xept commented 2 years ago

Re: trasnfer -> call -- Fixed in lifinance/lifi-contracts@274a41b047b3863d9ae232eefea04896dc32d853

H3xept commented 2 years ago

Re 2 step ownership transfer

Duplicate of #143