code-423n4 / 2024-03-zksync-findings

2 stars 1 forks source link

Upgraded Q -> 2 from #60 [1714762986225] #136

Closed c4-judge closed 7 months ago

c4-judge commented 7 months ago

Judge has assessed an item in Issue #60 as 2 risk. The relevant finding follows:

Low-05 StateTransitionManger cannot unfreeze a frozen chain to an error in StateTransitionManager.sol.

Instances(1) A state transition (ST)'s freezeDiamond() or unfreezeDiamond is intended to be called by either StateTransitionManger or the admin of the state transition. However, there is an error in unfreezeChain on StateTransitionManager.sol that will prevent StateTransitionManger from unfreezing a chain when needed.

//code/contracts/ethereum/contracts/state-transition/StateTransitionManager.sol
    function unfreezeChain(uint256 _chainId) external onlyOwner {
          //@audit this should be `unfreezeDiamond()`. This error prevents StateTransitionManager from freezing a chain(ST) when needed.
|>        IZkSyncStateTransition(stateTransition[_chainId]).freezeDiamond();
    }

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

A state transition(ST) can be malicious. The hyperchain implementation is based on the assumption that StateTransitionManager(STM) is the main point of trust. This error prevents STM to exercise key control over ST to unfreeze an ST. If the ST admin is malicious and intends to freeze the chain for longer, STM won't be able to exercise intended control of the malicious ST.

//code/contracts/ethereum/contracts/state-transition/chain-deps/facets/Admin.sol
      //@audit Due to error in StateTransitionManger, only ST admin can unfreeze the chain
|>    function unfreezeDiamond() external onlyAdminOrStateTransitionManager {
        Diamond.DiamondStorage storage diamondStorage = Diamond
            .getDiamondStorage();
        require(diamondStorage.isFrozen, "a7"); // diamond proxy is not frozen
        diamondStorage.isFrozen = false;
        emit Unfreeze();
    }

(https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/state-transition/chain-deps/facets/Admin.sol#L143)

Recommendations: In unfreezeChain, change to call unfreezeDiamond().

c4-judge commented 7 months ago

alex-ppg marked the issue as duplicate of #97

c4-judge commented 7 months ago

alex-ppg marked the issue as satisfactory