zkSync v2 contest - codehacker's results

Lines of code

https://github.com/code-423n4/2022-10-zksync/blob/main/ethereum/contracts/zksync/facets/DiamondCut.sol#L46 https://github.com/code-423n4/2022-10-zksync/blob/main/ethereum/contracts/zksync/facets/DiamondCut.sol#L61 https://github.com/code-423n4/2022-10-zksync/blob/main/ethereum/contracts/zksync/libraries/Diamond.sol#L92 https://github.com/code-423n4/2022-10-zksync/blob/main/ethereum/contracts/zksync/libraries/Diamond.sol#L113 https://github.com/code-423n4/2022-10-zksync/blob/main/ethereum/contracts/zksync/libraries/Diamond.sol#L282

Vulnerability details

Description

There is a function executeDiamondCutProposal in the DiamondCutFacet contract. It checks that proposal data passed as input to this call is equal to the data that is declared when the creation of this proposal using the following logic:

require(
    s.diamondCutStorage.proposedDiamondCutHash ==
        keccak256(abi.encode(_diamondCut.facetCuts, _diamondCut.initAddress)),
    "a4"
); // proposal should be created

But there are no checks on data that is used as input to the init contract during the execution of the diamond cut proposal.

Although such functionality may be in demand for the cases when part of the input data will be known only at the stage of execution of the proposal, in most cases init input data will be known at the stage of creation of the proposal, so it will be better to include such data to the proposal hash preimage. It will prevent a user from malicious input data that can be passed when executing the proposal and even from malicious input data that can be passed by an attacker through a potential leak of the governance private key (in the case when the governance private key was stolen attacker will not have an ability to maliciously run a fast upgrade and stole assets due to notice period, but in the case when trusted governance initiated the proposal, an attacker will have the ability to execute it with malicious init contract's input data).

Recommended Mitigation Steps

Use a combination of the data that is expected to be known at the stage of creation of the proposal (including such data as a part of the proposal hash preimage) with the data that is expected to be known only at the stage of the execution of the proposal.