zkSync Era - quarkslab's results

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/Constants.sol#L35 https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/Constants.sol#L27-L28 https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/AccountCodeStorage.sol#L93-L94

Vulnerability details

Impact

getCodeHash(address) function from AccountCodeStorage is called when the EXTCODEHASH EVM opcode is used. By reading this function, it appears that precompile contracts bytecode hash is EMPTY_STRING_KECCAK = 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470. The function returns this value for all addresses less than CURRENT_MAX_PRECOMPILE_ADDRESS, currently 0x02.

The problem is that there are 2 precompile contracts deployed at address 0x06 and 0x07. The bytecode hash value returned by getCodeHash(address) will be 0x00 for those two precompiles.

The main issue comes from CURRENT_MAX_PRECOMPILE_ADDRESS in Constants.sol, which is set to 0x02. As such, it doesn't include EcAdd (0x06) and EcMul (0x07) precompiles.

Proof of Concept

Affected code:

• https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/Constants.sol#L35
• https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/Constants.sol#L27-L28
• https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/AccountCodeStorage.sol#L93-L94

Scenario:

• A smart contract rely on the EXTCODEHASH to determine if a precompile exist before calling it
• Hash of precompile should be EMPTY_STRING_KECCAK
  • Smart contract will be able to call EcRecover and SHA256 precompiles
  • Smart contract will not be able to call EcAdd and EcMul precompiles

To verify the issue, the following contract can be used:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract ZigturLogPrecompiles {
    function returnHashFromPrecompiles(address _precompile) external view returns (bytes32 _value) {
        assembly {
            _value := extcodehash(_precompile)
        }
    }
}

Then, the following unit test can be used:

  it("Log precompiles hashes", async function () {
    const provider = new Provider(zkSyncTestnet.url);
    const wallet = new Wallet(RICH_WALLET_PK, provider);
    const deployer = new Deployer(hre, wallet);

    const LogPrecompilesArtifact = await deployer.loadArtifact('ZigturLogPrecompiles');
    const logPrecompilesContract = await deployer.deploy(LogPrecompilesArtifact);

    const EcRecoverPrecompileHash = await logPrecompilesContract.returnHashFromPrecompiles('0x0000000000000000000000000000000000000001');
    const Sha256PrecompileHash = await logPrecompilesContract.returnHashFromPrecompiles('0x0000000000000000000000000000000000000002');
    const EcAddPrecompileHash = await logPrecompilesContract.returnHashFromPrecompiles('0x0000000000000000000000000000000000000006');

    expect(EcRecoverPrecompileHash).to.equal('0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470');
    expect(Sha256PrecompileHash).to.equal('0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470');

    expect(EcAddPrecompileHash).not.equal('0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470');
    expect(EcAddPrecompileHash).to.equal('0x0000000000000000000000000000000000000000000000000000000000000000');
  });

Tools Used

Hardhat + zkSync docker local setup

Recommended Mitigation Steps

There are 2 solutions:

First solution

This solution consist of storing the EMPTY_STRING_KECCAK hash value in the AccountCodeStorage contract for each precompile.

This way, the precompiles check in getCodeHash could be deleted and the returned value would be valid.

Second solution

The second solution is to update CURRENT_MAX_PRECOMPILE_ADDRESS in Constants to set the new max precompile (0x07). Applying this solution isn't the best thing as not yet available precompiles (i.e. 0x03, 0x04 and 0x05) will have a non-zero hash value.

Assessed type

Context

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

Empty contracts hash doesn’t respect zkSync’s bytecode hash format The code hash returned by the getCodeHash function does not comply with zkSync’s bytecode hash format when querying an empty account, a constructing contract or a precompiled contract.

The getCodeHash function is supposed to simulate the behavior of the EXTCODEHASH EVM opcode, as per the EIP-1052 specification. However, zkSync redefined how bytecode hashes are defined:

“”" On zkSync the bytecode hashes are stored in the following format:

The 0th byte denotes the version of the format. Currently the only version that is used is “1”. The 1st byte is 0 for deployed contracts’ code and 1 for the contract code that is being constructed. The 2nd and 3rd bytes denote the length of the contract in 32-byte words as big-endian 2-byte number. The next 28 bytes are the last 28 bytes of the sha256 hash of the contract’s bytecode. The bytes are ordered in little-endian order (i.e. the same way as for bytes32 ). “”" getCodeHash returns the bytecode hash as stored in the AccountCodeStorage system contract (i.e. complying with zkSync’s definition of a bytecode hash), except for empty accounts, constructing contracts and precompiled contracts, for which it returns keccak256("") == 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470.

The valid value would be either 0x0100000086f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470 or 0x0101000086f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470 depending on whether the contract is constructing or not.

The impact is that smart contracts can’t rely on bytecode hash to know the bytecode size of an account. As the size in the hash is messed up for impacted accounts, smart contract will not consider those accounts as zero bytecode size ones. The decoded size from the hash would 0x4601 = 17921 instead of 0x0000.

Note: impacted accounts are empty, constructing and precompiled contracts.

Classification CWE-1068: Inconsistency Between Implementation and Documented Design