It's not possible to upgrade chainId

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/state-transition/StateTransitionManager.sol#L202-L217 https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/upgrades/BaseZkSyncUpgrade.sol#L147-L157

Vulnerability details

Impact

Function StateTransitionManager._setChainIdUpgrade() which is responsible for upgrading chainId, sets VerifierParams with the values which won't be accepted by BaseZkSyncUpgrade._setVerifierParams() function. Because of that - upgrading chainId won't be possible.

Proof of Concept

File: StateTransitionManager.sol

ProposedUpgrade memory proposedUpgrade = ProposedUpgrade({
            l2ProtocolUpgradeTx: l2ProtocolUpgradeTx,
            factoryDeps: bytesEmptyArray,
            bootloaderHash: bytes32(0),
            defaultAccountHash: bytes32(0),
            verifier: address(0),
            verifierParams: VerifierParams({
                recursionNodeLevelVkHash: bytes32(0),
                recursionLeafLevelVkHash: bytes32(0),
                recursionCircuitsSetVksHash: bytes32(0)
            }),
            l1ContractsUpgradeCalldata: new bytes(0),
            postUpgradeCalldata: new bytes(0),
            upgradeTimestamp: 0,
            newProtocolVersion: protocolVersion
        });

As demonstrated above, function StateTransitionManager._setChainIdUpgrade() sets VerifierParams to:

            verifierParams: VerifierParams({
                recursionNodeLevelVkHash: bytes32(0),
                recursionLeafLevelVkHash: bytes32(0),
                recursionCircuitsSetVksHash: bytes32(0)

However, those params won't be accepted by BaseZkSyncUpgrade._setVerifierParams() function.

File: BaseZkSyncUpgrade.sol

        if (
            _newVerifierParams.recursionNodeLevelVkHash == bytes32(0) &&
            _newVerifierParams.recursionLeafLevelVkHash == bytes32(0) &&
            _newVerifierParams.recursionCircuitsSetVksHash == bytes32(0)
        ) {
            return;
        }

        VerifierParams memory oldVerifierParams = s.verifierParams;
        s.verifierParams = _newVerifierParams;
        emit NewVerifierParams(oldVerifierParams, _newVerifierParams);

As demonstrated above, when either of the parameter is bytes32(0) - function BaseZkSyncUpgrade._setVerifierParams() will return, instead of continuing executing and updating VerifierParams.

The flow can be summarized as below:

1. StateTransitionManager._setChainIdUpgrade() sets recursionNodeLevelVkHash, recursionLeafLevelVkHash, recursionCircuitsSetVksHash to bytes32(0).
2. BaseZkSyncUpgrade._setVerifierParams() - lines 147-153: _newVerifierParams.recursionNodeLevelVkHash == bytes32(0) && _newVerifierParams.recursionLeafLevelVkHash == bytes32(0) && _newVerifierParams.recursionCircuitsSetVksHash == bytes32(0) condition is fulfilled, thus function returns, instead of upgrading VerifierParams.

This leads to the conclusion, that it's not possible to upgrade chainId, because StateTransitionManager._setChainIdUpgrade() sets VerifierParams to the bytes32(0), and according to BaseZkSyncUpgrade._setVerifierParams() - those parameters cannot be empty.

Tools Used

Manual code review

Recommended Mitigation Steps

When calling StateTransitionManager._setChainIdUpgrade(), make sure that VerifierParams (recursionNodeLevelVkHash, recursionLeafLevelVkHash, recursionCircuitsSetVksHash) are not empty and set them to the values accepted by BaseZkSyncUpgrade._setVerifierParams().

Assessed type

Invalid Validation

[saxenism]

Invalid, because just the verifierParams are not updated.

[c4-sponsor]

saxenism (sponsor) disputed

[razzorsec]

As demonstrated above, when either of the parameter is bytes32(0) - function BaseZkSyncUpgrade._setVerifierParams()

1) The function will return, when all of the parameters are bytes32(0). 2) The function _setVerifierParams() is a private function called within the _upgradeVerifier parent internal function frame, hence on return, the function will jump back to its parent function frame. The execution will still continue.

[alex-ppg]

The Warden specifies that a chain ID upgrade may fail when a single empty verifier parameter is configured.

As the Sponsor claims and the code confirms, all parameters will have to be 0 for the early return to trigger in verifier parameter configurations. Additionally, the return does not "bubble up" and cause execution to halt; the BaseZkSyncUpgrade::_setVerifierParams function will simply return early and the code will continue execution as expected. As such, this exhibit is considered invalid.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid