zkSync Era - minhtrng's results

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/contracts/ethereum/contracts/zksync/libraries/TransactionValidator.sol#L41

Vulnerability details

Impact

An incorrect check allows an L1->L2 transaction to be sent that does not cover the sum of overhead and intrinsic costs for the operator.

Proof of Concept

The gasLimit required for a transaction consists of (see docs):

$\text{totalGasLimit} = \text{overhead + actualGasLimit} = \text{overhead + (intrinisicCosts + executionCosts)}$

The function TransactionValidator.getMinimalPriorityTransactionGasLimit calculates the intrinsic costs that will be incurred for the processing of a transaction on L2. The calculated value is checked in TransactionValidator.validateL1ToL2Transaction like this:

require(
    getMinimalPriorityTransactionGasLimit(
        _encoded.length,
        _transaction.factoryDeps.length,
        _transaction.gasPerPubdataByteLimit
    ) <= _transaction.gasLimit, 
    "up"
);

The issue is that _transaction.gasLimit is the total gasLimit including the overhead for the operator. The overhead costs are already checked before that in TransactionValidator.getTransactionBodyGasLimit

function getTransactionBodyGasLimit(
    uint256 _totalGasLimit,
    uint256 _gasPricePerPubdata,
    uint256 _encodingLength
) internal pure returns (uint256 txBodyGasLimit) {
    uint256 overhead = getOverheadForTransaction(_totalGasLimit, _gasPricePerPubdata, _encodingLength);

    require(_totalGasLimit >= overhead, "my"); // provided gas limit doesn't cover transaction overhead
    unchecked {
        txBodyGasLimit = _totalGasLimit - overhead;
    }
}

The value returned by this function is the actualGasLimit (see formula above) that will be available for the processing of the transaction and which should be used to check if the intrinsic costs are covered.

In the current implementation the totalGasLimit is checked twice if its greater than overhead and intrinsic costs (separately, not the sum). The bootloader does things correctly by subtracting the overhead from the totalGasLimit first and then checking if the rest covers the intrinsic costs (also called "preparation" costs):

function processL1Tx(...){
    ...
    //gasLimitForTx is total - overhead (and some other intrinsic costs)
    let gasLimitForTx, reservedGas := getGasLimitForTx(...)
    ...
    canonicalL1TxHash, gasUsedOnPreparation := l1TxPreparation(txDataOffset)
    ...
}   if gt(gasLimitForTx, gasUsedOnPreparation) {
        ...
        potentialRefund, success := getExecuteL1TxAndGetRefund(txDataOffset, sub(gasLimitForTx, gasUsedOnPreparation))

That means users will not be able to execute transactions for cheap. However, since L1->L2 transaction have to be processed (due to the way the priority queue works), it would be possible to grief the operator by submitting transactions that only cover $\text{max(overhead, intrinsicCosts)}$. Those transactions would not have enough gas to be executed on L2, but the overhead and intrinsic costs would still be incurred.

Tools Used

Manual Review

Recommended Mitigation Steps

In TransactionValidator.validateL1ToL2Transaction change the check like this:

uint256 l2GasForTxBody = getTransactionBodyGasLimit(...)
...
require(
    getMinimalPriorityTransactionGasLimit(
        _encoded.length,
        _transaction.factoryDeps.length,
        _transaction.gasPerPubdataByteLimit
    ) <= l2GasForTxBody, // <---
    "up"
);

l2GasForTxBody is what remains after subtracting the overhead from the totalGasLimit.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/bootloader/bootloader.yul#L1847 https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/contracts/ethereum/contracts/zksync/libraries/TransactionValidator.sol#L145

Vulnerability details

Impact

The overhead can be calculated incorrectly for some transactions and the user will get charged 32 times more in those cases than what he ought to be charged.

Proof of Concept

The overhead is calculated as the maximum of:

1. overhead for slot
2. overhead for memory occupation in the bootloader
3. overhead for single instance circuits

2. is calculated like this:

$\text{MemoryOverhead} = \frac{\text{BatchOverhead} * \text{EncodingLength}}{\text{BootloaderMemory}}$

In other words, the overhead for 2. is proportional to the percentage of bootloader memory that the transaction will use.

The issue is that in the actual implementation the units used for EncodingLength and BootloaderMemory are different. EncodingLength is denominated in bytes, while BootloaderMemory is denominated in words (32 bytes).

Looking at the implementations:

//TransactionValidator.getOverheadForTransaction
uint256 overheadForLength = Math.ceilDiv(_encodingLength * batchOverheadGas, BOOTLOADER_TX_ENCODING_SPACE);

//bootloader.getTransactionUpfrontOverhead
let overheadForLength := ceilDiv(
    safeMul(txEncodeLen, totalBatchOverhead, "ad"),
    BOOTLOADER_MEMORY_FOR_TXS()
)

_encodingLength/txEncodeLen are a result of abi.encode(transaction).length and are hence expressed in bytes. BOOTLOADER_TX_ENCODING_SPACE/BOOTLOADER_MEMORY_FOR_TXS() have the value 273132 in the current config. While the specific number may be subject to change, it is relevant that it is not expressed in bytes, but in words (32 bytes each).

The number 273132 is derived from the current occupation of the word indices [250141...523261] (difference is slightly off with 273120). Note that this range is derived from my own tracing of the bootloaders memory slots with the current config, so there might be a minor error in either the configuration or my own attempt of enumerating the slots. The docs state that [252189..523261] should be the range (difference 271072). Either way the minor difference is negligible and does not affect the stated #Impact. Due to the different units used, the denominator in above formula is 32 times smaller than it should be, hence calculating a 32 times larger overhead

Tools Used

Manual Review

Recommended Mitigation Steps

Multiply BOOTLOADER_TX_ENCODING_SPACE and BOOTLOADER_MEMORY_FOR_TXS() by 32 when using them in the calculation. Alternatively, change those values to be in bytes from the get-go, which should be fine as they dont seem to be used anywhere else currently

Assessed type

Other