In the walkthrough video, it said that the upgrades of Diamond must go through a proposal window with a delay of 7 days. Upgrade should be done by first call proposeDiamondCut and then wait 7 days and call diamondCut.
But this timelock can be bypassed because the check if it passed 7 days is wrong. It just check acceptanceTimes < block.timestamp. If owner not call proposeDiamondCut then acceptanceTimes is default value which is 0 and the check will always pass.
This is very dangerous when admin can use all privileges without a delay, including withdraw all the funds.
Please refer to this issue to check the severity when timelock can be bypassed.
Proof of Concept
Contract owner call diamondCut() with any parameters without proposing it first.
Because it is not proposed, its acceptanceTimes is 0 by default and the check will passed.
LayneHaber
commented
2 years ago
Lines of code
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/libraries/LibDiamond.sol#L101
Vulnerability details
Impact
In the walkthrough video, it said that the upgrades of Diamond must go through a proposal window with a delay of 7 days. Upgrade should be done by first call
proposeDiamondCutand then wait 7 days and calldiamondCut.But this timelock can be bypassed because the check if it passed 7 days is wrong. It just check
acceptanceTimes < block.timestamp. If owner not callproposeDiamondCutthenacceptanceTimesis default value which is 0 and the check will always pass.This is very dangerous when admin can use all privileges without a delay, including withdraw all the funds.
Please refer to this issue to check the severity when timelock can be bypassed.
Proof of Concept
diamondCut()with any parameters without proposing it first.acceptanceTimesis 0 by default and the check will passed.Tools Used
Manual Review
Recommended Mitigation Steps
Add check if
acceptanceTimes > 0