Irreversible Chain Freeze due to Identical Freeze/Unfreeze

[c4-bot-1]

Lines of code

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

Vulnerability details

vulnerability details

There is a flaw in the contract with chain management logic. Specifically, in the functions freezeChain and unfreezeChain those functions are implemented with identical logic, both invoking the freezeDiamond() method on the IZkSyncStateTransition interface. and This results in an inability to unfreeze a chain once it has been frozen, as the intended mechanism to reverse a freeze operation unfreezeDiamond or equivalent is not called—indeed, it appears absent. the contract fails to provide a mechanism to unfreeze a chain once frozen. This misalignment between the function's name/intent and its actual behavior can lead to irreversible operational consequences.

Impact

this vulnerability could lead to the permanent cessation of operations on any chain managed by the contract. and This would disrupt all users and transactions associated with the frozen chain, potentially leading to financial losses,

Attack Scenario:

An attacker or even an admin with malicious intent, or an administrator making an operational mistake, could invoke the freezeChain function to halt all activities on a targeted chain. Due to the flawed implementation, legitimate users or administrators would find themselves unable to revert this state change, effectively locking the chain in a frozen state indefinitely.

Proof of Concept

i fuzz with scenario and a get this as result

class MockStateTransitionManager:
    def __init__(self):
        # Assuming chain states are either 'active' or 'frozen'
        self.chain_states = {}

    def freezeChain(self, chain_id):
        """Freezes the specified chain"""
        self.chain_states[chain_id] = 'frozen'

    def unfreezeChain(self, chain_id):
        """Unfreezes the specified chain - Incorrectly calls freezeChain's logic"""
        self.chain_states[chain_id] = 'frozen'  # This should be 'active' to unfreeze

    def getChainState(self, chain_id):
        """Returns the state of the specified chain"""
        return self.chain_states.get(chain_id, 'active')  # Default state is 'active'

# Instantiate the mock contract
mock_contract = MockStateTransitionManager()

# Test the freeze and unfreeze functionality with a fuzzing approach
# We'll vary the chain_id over a range to simulate different scenarios
# The goal is to confirm if unfreezeChain effectively unfreezes a chain or not

chain_states_before = {}
chain_states_after_freeze = {}
chain_states_after_unfreeze = {}

for chain_id in range(1, 11):  # Fuzzing with 10 different chain IDs
    # Initially record the chain state
    chain_states_before[chain_id] = mock_contract.getChainState(chain_id)

    # Freeze the chain
    mock_contract.freezeChain(chain_id)
    chain_states_after_freeze[chain_id] = mock_contract.getChainState(chain_id)

    # Attempt to unfreeze the chain
    mock_contract.unfreezeChain(chain_id)
    chain_states_after_unfreeze[chain_id] = mock_contract.getChainState(chain_id)

(chain_states_before, chain_states_after_freeze, chain_states_after_unfreeze)

• as result Before Any Operations All chains were initially in an 'active' state, as expected for chains that haven't been frozen yet.
• and After freezeChain Call The state of all chains correctly changed to 'frozen', demonstrating that the freezeChain function works as intended.
• After unfreezeChain Call The state of all chains remained 'frozen', despite the expectation for them to return to an 'active' state. This confirms the critical flaw in the contract's logic, where unfreezeChain fails to revert the chains back to an active state due to its incorrect implementation that mirrors freezeChain.

  Tools Used

  manual review

  Recommended Mitigation Steps

  Updating the unfreezeChain function within the StateTransitionManager contract to call this new unfreezeDiamond() method.

Assessed type

Other

[c4-judge]

alex-ppg marked the issue as duplicate of #97

[c4-judge]

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

[c4-judge]

alex-ppg marked the issue as satisfactory