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

0 stars 0 forks source link

Missing timelock for critical contract setters of privileged roles #72

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0xRajeev

Vulnerability details

Impact

Setter functions for critical protocol parameters accessible only by privileged roles e.g. onlyOwner should consider adding timelocks so that users and other privileged roles (in the case of a multiSig) can detect upcoming changes and have the time to react to them.

Changes ownership, minRate, maxRate, lowRate and targetUtilization 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.

None of the setters in the protocol have a timelock controller functionality to delay enforcement of the critical changes they make.

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/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/external/Ownable.sol#L29-L33

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/InterestRateModel.sol#L43-L47

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/InterestRateModel.sol#L49-L53

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/InterestRateModel.sol#L55-L58

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/InterestRateModel.sol#L60-L64

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/LPTokenMaster.sol#L43-L48

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider adding timelocks to such contracts with critical setter functions.

talegift commented 2 years ago

There s nothing in the code suggesting that we either will or will not use a timelock.

Timelock will be used as the owner of all key contracts. This is also not a bug in the code.

As per the pre the judge's comment on a similar issue from the previous audit.

This is a risk for users that they have to take. I don't see a protocol exploit here so making this invalid.

https://github.com/code-423n4/2021-07-wildcredit-findings/issues/93#issuecomment-890592171

ghoul-sol commented 2 years ago

I stand by my judgement. Invalid.