search

code-423n4 / 2021-09-sushitrident-findings

0 stars 0 forks source link

Missing timelock for critical contract setters of privileged roles #55

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

Setter functions for critical contract parameters accessible only by privileged roles e.g. admin should consider adding timelocks (along with emitted events) so that users and other privileged roles can detect upcoming changes and have the time to react to them.

Changes to whitelists, barFee and migrator address may have a financial or trust impact on users who should be given an opportunity to react to them by exiting/engaging without being surprised when such changes are made effective immediately.

See similar Medium-severity finding in ConsenSys's Audit of 1inch Liquidity Protocol (https://consensys.net/diligence/audits/2020/12/1inch-liquidity-protocol/#unpredictable-behavior-for-users-due-to-admin-front-running-or-general-bad-timing)

Proof of Concept

https://github.com/sushiswap/trident/blob/6bd4c053b6213ffc612987eb565aa3813d5f0d13/contracts/deployer/MasterDeployer.sol#L49-L57

https://github.com/sushiswap/trident/blob/6bd4c053b6213ffc612987eb565aa3813d5f0d13/contracts/deployer/MasterDeployer.sol#L59-L63

https://github.com/sushiswap/trident/blob/6bd4c053b6213ffc612987eb565aa3813d5f0d13/contracts/deployer/MasterDeployer.sol#L65-L68

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider adding timelocks to such contracts with critical setter functions.

maxsam4 commented 3 years ago

Acceptable risk. There is no critical function controlled by the owner.