c4-bot-1 commented 8 months ago

Lines of code

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

Vulnerability details

The StateTransitionManager contract in the zkSync system contains two functions, freezeChain and unfreezeChain, intended to manage the operational state of a specified blockchain by freezing and unfreezing it, respectively. Both functions are designed to either halt or resume operations, particularly in response to detected vulnerabilities or the need for maintenance. However, unfreezeChain() function incorrectly calls freezeDiamond() instead of an expected counterpart method unfreezeDiamond() that would revert the chain's state. This means that chain that is once frozen, is blocked forever.

Impact

Permament DoS of a chain after first freeze.

Proof of Concept

Please add following test to FreezeChain.t.sol and execute it:

    function test_FreezingUnfreezingChain() public {
        createNewChain(getDiamondCutData(diamondInit));

        address newChainAddress = chainContractAddress.stateTransition(chainId);
        GettersFacet gettersFacet = GettersFacet(newChainAddress);
        bool isChainFrozen = gettersFacet.isDiamondStorageFrozen();
        assertEq(isChainFrozen, false);

        vm.stopPrank();
        vm.startPrank(governor);

        chainContractAddress.freezeChain(block.chainid);
        vm.expectRevert(bytes.concat("q1")); // storage frozen, cannot unfreeze
        chainContractAddress.unfreezeChain(block.chainid);
    }

The tests runs successfully, meaning that it reverts with q1 error - storage frozen:

Running 1 test for test/foundry/unit/concrete/state-transition/StateTransitionManager/FreezeChain.t.sol:freezeChainTest
[PASS] test_FreezingUnfreezingChain() (gas: 3397331)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.46ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Manual Review

Recommended Mitigation Steps

Change code to unfreeze diamond:

function unfreezeChain(uint256 _chainId) external onlyOwner {
-    IZkSyncStateTransition(stateTransition[_chainId]).freezeDiamond();
+    IZkSyncStateTransition(stateTransition[_chainId]).unfreezeDiamond();
}

Assessed type

DoS

c4-judge commented 8 months ago

alex-ppg marked the issue as duplicate of #97

c4-judge commented 7 months ago

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