Permanent freezing of an hyperchain if it is ever freezed by the `StateTransitionManager`

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/state-transition/StateTransitionManager.sol#L166

Vulnerability details

Impact

There is a typo when unfreezing an hyperchain, which makes it impossible to unfreeze a previously freezed one.

Proof of Concept

Pretty visual

StateTransitionManager, function unfreezeChain

    /// @dev freezes the specified chain
    function unfreezeChain(uint256 _chainId) external onlyOwner {
        IZkSyncStateTransition(stateTransition[_chainId]).freezeDiamond(); // @audit must be unfreezeDiamond(), typo
    }

Calling unfreezeChain on a freezed one will always revert due to the following require in

Admin, function freezeDiamond

    function freezeDiamond() external onlyAdminOrStateTransitionManager {
        ...

        require(!diamondStorage.isFrozen, "a9"); // diamond proxy is frozen already

        ...
    }

Runnable POC (for completeness)

    function test_freezeX2() public {
        address admin = utilsFacet.util_getAdmin();

        vm.expectRevert(bytes.concat("a9"));

        vm.startPrank(admin);
        adminFacet.freezeDiamond();
        adminFacet.freezeDiamond();
        vm.stopPrank();
    }

Recommended Mitigation Steps

Trivial

StateTransitionManager, function unfreezeChain

    /// @dev freezes the specified chain
    function unfreezeChain(uint256 _chainId) external onlyOwner {
-       IZkSyncStateTransition(stateTransition[_chainId]).freezeDiamond();
+       IZkSyncStateTransition(stateTransition[_chainId]).unfreezeDiamond();
    }

Assessed type

Other

[c4-judge]

alex-ppg marked the issue as duplicate of #97

[c4-sponsor]

saxenism (sponsor) confirmed

[c4-sponsor]

saxenism marked the issue as disagree with severity

[saxenism]

Thank you for the finding.

We, however think this is a medium severity issue since in the current codebase admin could also unfreeze.

[c4-judge]

alex-ppg changed the severity to 2 (Med Risk)

[c4-judge]

alex-ppg marked the issue as partial-75

[c4-judge]

alex-ppg marked the issue as satisfactory