search

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

6 stars 4 forks source link

OnlyOwner functions that make critical changes should have a timelock. #74

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/DexManagerFacet.sol#L44 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/DexManagerFacet.sol#L62

Vulnerability details

Impact

removeDex(), batchRemoveDex() should have a timelock. Such functions that make critical changes should have a timelock, so that users can see the upcoming changes and have time to react to these changes.

Proof of Concept

See below for similar findings: https://consensys.net/diligence/audits/2020/12/1inch-liquidity-protocol/#unpredictable-behavior-for-users-due-to-admin-front-running-or-general-bad-timing https://github.com/code-423n4/2021-11-overlay-findings/issues/120

Tools Used

Manual analysis

Recommended Mitigation Steps

Add timelock for critical changes which will effect user experience and decisions.

H3xept commented 2 years ago

Our team is currently focusing on creating a stable and trustworthy system as fast as possible. We agree with the increased safety a DAO/Multisign mechanism and will provide them in the future. Timelocks are currently not planned, as we want to be able to react fast if we have to disable bridges for security reasons (e.g. if the underlying bridge is being exploited).

gzeoneth commented 2 years ago

Duplicate of https://github.com/code-423n4/2022-03-lifinance-findings/issues/65