The enforcement of the maximum amount of L2 gas that should be spent on a transaction is flawed as it allows the gas spent to be more than the `Operator's trust limit`

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/system-contracts/bootloader/bootloader.yul#L919-L1052

Vulnerability details

Proof of Concept

First take a look at https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/system-contracts/bootloader/bootloader.yul#L919-L1052

            function processL1Tx(
                txDataOffset,
                resultPtr,
                transactionIndex,
                gasPerPubdata,
                isPriorityOp
            ) {
                // For L1->L2 transactions we always use the pubdata price provided by the transaction.
                // This is needed to ensure DDoS protection. All the excess expenditure
                // will be refunded to the user.
                // Skipping the first formal 0x20 byte
                let innerTxDataOffset := add(txDataOffset, 32)

                let basePubdataSpent := getPubdataCounter()

                let gasLimitForTx, reservedGas := getGasLimitForTx(
                    innerTxDataOffset,
                    transactionIndex,
                    gasPerPubdata,
                    L1_TX_INTRINSIC_L2_GAS(),
                    L1_TX_INTRINSIC_PUBDATA()
                )

                let gasUsedOnPreparation := 0
                let canonicalL1TxHash := 0

                canonicalL1TxHash, gasUsedOnPreparation := l1TxPreparation(txDataOffset, gasPerPubdata, basePubdataSpent)

                let refundGas := 0
                let success := 0

                // The invariant that the user deposited more than the value needed
                // for the transaction must be enforced on L1, but we double check it here
                let gasLimit := getGasLimit(innerTxDataOffset)

                // Note, that for now the property of block.base <= tx.maxFeePerGas does not work
                // for L1->L2 transactions. For now, these transactions are processed with the same gasPrice
                // they were provided on L1. In the future, we may apply a new logic for it.
                let gasPrice := getMaxFeePerGas(innerTxDataOffset)
                let txInternalCost := safeMul(gasPrice, gasLimit, "poa")
                let value := getValue(innerTxDataOffset)
                if lt(getReserved0(innerTxDataOffset), safeAdd(value, txInternalCost, "ol")) {
                    assertionError("deposited eth too low")
                }

                // In previous steps, there might have been already some pubdata published (e.g. to mark factory dependencies as published).
                // However, these actions are mandatory and it is assumed that the L1 Mailbox contract ensured that the provided gas is enough to cover for pubdata.

                if gt(gasLimitForTx, gasUsedOnPreparation) {
                    let gasSpentOnExecution := 0
                    let gasForExecution := sub(gasLimitForTx, gasUsedOnPreparation)

                    gasSpentOnExecution, success := getExecuteL1TxAndNotifyResult(
                        txDataOffset,
                        gasForExecution,
                        basePubdataSpent,
                        gasPerPubdata,
                    )

                    let ergsSpentOnPubdata := getErgsSpentForPubdata(
                        basePubdataSpent,
                        gasPerPubdata
                    )

                    // It is assumed that `isNotEnoughGasForPubdata` ensured that the user did not publish too much pubdata.
                    let potentialRefund := saturatingSub(
                        safeAdd(reservedGas, gasForExecution, "safeadd: potentialRefund1"),
                        safeAdd(gasSpentOnExecution, ergsSpentOnPubdata, "safeadd: potentialRefund2")
                    )

                    // Asking the operator for refund
                    askOperatorForRefund(potentialRefund, ergsSpentOnPubdata, gasPerPubdata)

                    // In case the operator provided smaller refund than the one calculated
                    // by the bootloader, we return the refund calculated by the bootloader.
                    refundGas := max(getOperatorRefundForTx(transactionIndex), potentialRefund)
                }

                //@audit

                if gt(refundGas, gasLimit) {
                    assertionError("L1: refundGas > gasLimit")
                }

                let payToOperator := safeMul(gasPrice, safeSub(gasLimit, refundGas, "lpah"), "mnk")

                notifyAboutRefund(refundGas)

                // Paying the fee to the operator
                mintEther(BOOTLOADER_FORMAL_ADDR(), payToOperator, false)

                let toRefundRecipient
                switch success
                case 0 {
                    if iszero(isPriorityOp) {
                        // Upgrade transactions must always succeed
                        assertionError("Upgrade tx failed")
                    }

                    // If the transaction reverts, then minting the msg.value to the user has been reverted
                    // as well, so we can simply mint everything that the user has deposited to
                    // the refund recipient
                    toRefundRecipient := safeSub(getReserved0(innerTxDataOffset), payToOperator, "vji")
                }
                default {
                    // If the transaction succeeds, then it is assumed that msg.value was transferred correctly. However, the remaining
                    // ETH deposited will be given to the refund recipient.

                    toRefundRecipient := safeSub(getReserved0(innerTxDataOffset), safeAdd(getValue(innerTxDataOffset), payToOperator, "kpa"), "ysl")
                }

                if gt(toRefundRecipient, 0) {
                    let refundRecipient := getReserved1(innerTxDataOffset)
                    // Zero out the first 12 bytes to be sure that refundRecipient is address.
                    // In case of an issue in L1 contracts, we still will be able to process tx.
                    refundRecipient := and(refundRecipient, sub(shl(160, 1), 1))
                    mintEther(refundRecipient, toRefundRecipient, false)
                }

                mstore(resultPtr, success)

                debugLog("Send message to L1", success)

                // Sending the L2->L1 log so users will be able to prove transaction execution result on L1.
                sendL2LogUsingL1Messenger(true, canonicalL1TxHash, success)

                if isPriorityOp {
                    // Update priority txs L1 data
                    mstore(0, mload(PRIORITY_TXS_L1_DATA_BEGIN_BYTE()))
                    mstore(32, canonicalL1TxHash)
                    mstore(PRIORITY_TXS_L1_DATA_BEGIN_BYTE(), keccak256(0, 64))
                    mstore(add(PRIORITY_TXS_L1_DATA_BEGIN_BYTE(), 32), add(mload(add(PRIORITY_TXS_L1_DATA_BEGIN_BYTE(), 32)), 1))
                }
            }

This is the updated method of processing L1 transactions in the bootloader, and in comparison to the previous implementation there are notable improvements including the implicit setting of pubdata prices, advanced calculation of basePubdataSpent for more precise gas and refund assessments, and a more complex approach to calculating potential refunds, among others.

Now keep in mind that the reservedGas is the number of L2 gas that is beyond the MAX_GAS_PER_TRANSACTION and beyond the operator's trust limit , i.e. gas which cannot be allowed to be used for the transaction and is to be refunded, as clearly documented in the docs and commented here: https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/system-contracts/bootloader/bootloader.yul#L1214-L1271

Now issue with the logic applied to processL1Tx() is that it allows for part of this reservedGas to be used on the execution if not all.

While executing processL1Tx()the function getExecuteL1TxAndNotifyResult() is used to execute the L1 tx, also would be key to note that as the name hints the getExecuteL1TxAndNotifyResult() function does not return the potentialRefund value but rather only executes the transaction, notifies the result whether successful or not, while returning the gasSpentOnExecution value, see https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/system-contracts/bootloader/bootloader.yul#L1053-L1076

            function getExecuteL1TxAndNotifyResult(
                txDataOffset,
                gasForExecution,
                basePubdataSpent,
                gasPerPubdata
            ) -> gasSpentOnExecution, success {
                debugLog("gasForExecution", gasForExecution)

                let callAbi := getNearCallABI(gasForExecution)
                debugLog("callAbi", callAbi)

                checkEnoughGas(gasForExecution)

                let gasBeforeExecution := gas()
                success := ZKSYNC_NEAR_CALL_executeL1Tx(
                    callAbi,
                    txDataOffset,
                    basePubdataSpent,
                    gasPerPubdata
                )
                notifyExecutionResult(success)
                gasSpentOnExecution := sub(gasBeforeExecution, gas())
            }

Back to processL1Tx() we can see how the potentialRefund https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/system-contracts/bootloader/bootloader.yul#L985-L989

                    let potentialRefund := saturatingSub(
                        safeAdd(reservedGas, gasForExecution, "safeadd: potentialRefund1"),
                        safeAdd(gasSpentOnExecution, ergsSpentOnPubdata, "safeadd: potentialRefund2")
                    )

Evidently, this function erroneously subtracts both gasSpentOnExecution & ergsSpentOnPubdata from the summation of reservedGas & gasForExecution whereas it should only subtract from the gasForExecution.

To show that the above statement is true and that subtracting from the summation of reservedGas & gasForExecution consider the next three paragraphs.

Note that previous to this, both in processL1Tx() and getExecuteL1TxAndNotifyResult() no where is it enforced that the gas to be spent on execution is less than the available gasForExecution.

Now considering this, from the getGasLimitForTx's function implementation we see that gasLimitForTx is the maximum number of L2 gas that should be spent on that transaction, while atleast the reservedGas or more should be refunded to the operator.

Now going down the line of processL1Tx(), after querying the value of gasLimitForTx from getGasLimitForTx, it then calculates the gas meant for execution as a subtraction of the gas used on the tx preparation from the gasLimitForTx, here https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/system-contracts/bootloader/bootloader.yul#L970-L971

Impact

Core logic of having trust limits for operators is flawed.

This is cause the gas spent on the transaction could be higher than the maximum specified L2 gas for the transaction gasLimitForTx essentially meaning that the subtle invariant of having a trust limit for an operator on the amount of gas that can be spent on a transaction is not properly enforced, currently the refunded gas for these transaction could be less than the transaction's supposed reservedGas.

Recommended Mitigation Steps

The gasSpentOnExecution + ergsSpentOnPubdata value should be checked with only the gasForExecution, gasForExecution > (gasSpentOnExecution + ergsSpentOnPubdata) should be enforced.

And then potentialRefund should also be enforced to always be >= reservedGas, potentially with the difference from "gasForExecution - (gasSpentOnExecution + ergsSpentOnPubdata)", i.e when the difference > 0.

Assessed type

Context

[c4-sponsor]

saxenism (sponsor) disputed

[saxenism]

This looks like a misunderstanding of the purpose of the reservedGas. It still should be refunded in full.

[alex-ppg]

The Warden specifies that a subtraction when calculating the potential refund at the bootloader level is incorrect.

The Sponsor specifies that the submission is incorrect, however, they focused on an incorrect aspect of the exhibit. I discussed this with the Sponsor directly, and after investigating, we concluded that the exhibit was indeed invalid.

The gasForExecution variable is guaranteed to be greater than or equal to the gasSpentOnExecution + ergsSpentOnPubdata sum due to the unique nature of the ZKSYNC_NEAR_CALL_executeL1Tx function. An invocation of the function creates a new call frame whose gas limit (i.e. gas()) is set to the ABI's value. Based on this line, the value is set to the gasForExecution variable's value.

As the actual gas consumed during execution is evaluated as the delta between gas() invocations, the maximum gasSpentOnExecution value is going to be whatever ZKSYNC_NEAR_CALL_executeL1Tx ends up consuming, which is up-to gasForExecution.

The ZKSYNC_NEAR_CALL_executeL1Tx function will invoke the isNotEnoughGasForPubdata function at its end, supplying the gas() result as an argument to it. The isNotEnoughGasForPubdata function will ensure that the gas() in the context of the ZKSYNC_NEAR_CALL_executeL1Tx at its end is greater than or equal to the ergsSpentOnPubdata value, as evidenced here.

Culminating the above, we have the following relations:

• gas() at beginning of ZKSYNC_NEAR_CALL_executeL1Tx: gasForExecution
• gas() at end of ZKSYNC_NEAR_CALL_executeL1Tx: someValue >= ergsSpentOnPubdata

Based on the above, we know that the delta between gas() evaluates before and after the ZKSYNC_NEAR_CALL_executeL1Tx is going to be a value for which the following will hold true:

$$ gasForExecution - gasSpentOnExecution >= ergsSpentOnPubdata $$

Taking the above equation one step further, we have:

$$ gasForExecution >= gasSpentOnExecution + ergsSpentOnPubdata $$

As a result, the Warden's assumption that the gasForExecution may not cover the gasSpentOnExecution + ergsSpentOnPubdata value required is invalid.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid