code-423n4 / 2023-03-zksync-findings

6 stars 1 forks source link

Attacker can use L1->L2 transactions to register factory deps for free #69

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/main/bootloader/bootloader.yul#L912

Vulnerability details

Impact

The bootloader's execution of L1->L2 transactions allows an attacker to register factory dependencies without paying any gas fees on L2.

Currently, L1->L2 transactions are already free. Even if they are not, it's not clear whether the gas costs you save on L2 make up for the costs of the transaction on L1. Nevertheless, I thought that this might be unexpected behavior in the bootloader since there's no documentation on it.

Proof of Concept

First, some knowledge of the bootloader's gas operations I've collected through discussions with the sponsor (Vlad & Barichek):

The one place where nearCall isn't used is in l1TxPreparation(), part of processL1Tx():

            function l1TxPreparation(txDataOffset) -> canonicalL1TxHash, gasUsedOnPreparation {
                let innerTxDataOffset := add(txDataOffset, 0x20)

                let gasBeforePreparation := gas()
                debugLog("gasBeforePreparation", gasBeforePreparation)

                // Even though the smart contracts on L1 should make sure that the L1->L2 provide enough gas to generate the hash
                // we should still be able to do it even if this protection layer fails.
                canonicalL1TxHash := getCanonicalL1TxHash(txDataOffset)
                debugLog("l1 hash", canonicalL1TxHash)

                markFactoryDepsForTx(innerTxDataOffset, true)

                gasUsedOnPreparation := safeSub(gasBeforePreparation, gas(), "xpa")
                debugLog("gasUsedOnPreparation", gasUsedOnPreparation)
            }

            function markFactoryDepsForTx(innerTxDataOffset, isL1Tx) {
                debugLog("starting factory deps", 0)
                let factoryDepsPtr := getFactoryDepsPtr(innerTxDataOffset)
                let factoryDepsLength := mload(factoryDepsPtr)

                if gt(factoryDepsLength, MAX_NEW_FACTORY_DEPS()) {
                    assertionError("too many factory deps")
                }

                let ptr := NEW_FACTORY_DEPS_BEGIN_BYTE()
                // Selector
                mstore(ptr, {{MARK_BATCH_AS_REPUBLISHED_SELECTOR}})
                ptr := add(ptr, 32)

                // Saving whether the dependencies should be sent on L1
                // There is no need to send them for L1 transactions, since their
                // preimages are already available on L1.
                mstore(ptr, iszero(isL1Tx))
                ptr := add(ptr, 32)

                // Saving the offset to array (it is always 64)
                mstore(ptr, 64)
                ptr := add(ptr, 32)

                // Saving the array

                // We also need to include 32 bytes for the length itself
                let arrayLengthBytes := safeAdd(32, safeMul(factoryDepsLength, 32, "ag"), "af")
                // Copying factory deps array
                memCopy(factoryDepsPtr, ptr, arrayLengthBytes)

                let success := call(
                    gas(),
                    KNOWN_CODES_CONTRACT_ADDR(),
                    0,
                    // Shifting by 28 to start from the selector
                    add(NEW_FACTORY_DEPS_BEGIN_BYTE(), 28),
                    // 4 (selector) + 32 (send to l1 flag) + 32 (factory deps offset)+ 32 (factory deps length)
                    safeAdd(100, safeMul(factoryDepsLength, 32, "op"), "ae"),
                    0,
                    0
                )

                debugLog("factory deps success", success)

                if iszero(success) {
                    debugReturndata()
                    switch isL1Tx 
                    case 1 {
                        revertWithReason(
                            FAILED_TO_MARK_FACTORY_DEPS(),
                            1
                        )
                    }
                    default {
                        // For L2 transactions, we use near call panic
                        nearCallPanic()
                    }
                }
            }

There, the bootloader executes markFactoryDepsForTx() where the KnownCodesStorage contract is called to register all the factory dependencies of the current transaction. None of that is done with a nearCall. So, there's no limit set on the amount of gas that can be used.

l1TxPreparation() is part of processL1Tx():

            function processL1Tx(
                txDataOffset,
                resultPtr,
                transactionIndex,
                gasPerPubdata
            ) {
                // Skipping the first formal 0x20 byte
                let innerTxDataOffset := add(txDataOffset, 0x20) 

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

                let gasUsedOnPreparation := 0
                let canonicalL1TxHash := 0

                // @audit unbounded gas usage. Total gas usage is 2nd return value
                canonicalL1TxHash, gasUsedOnPreparation := l1TxPreparation(txDataOffset)

                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")
                }

                // @audit if this is false (user can just set very low gas limit) we skip the tx
                // execution. It will simply be saved as unsuccessful
                if gt(gasLimitForTx, gasUsedOnPreparation) {
                    let potentialRefund := 0

                    potentialRefund, success := getExecuteL1TxAndGetRefund(txDataOffset, sub(gasLimitForTx, gasUsedOnPreparation))

                    // Asking the operator for refund
                    askOperatorForRefund(potentialRefund)

                    // 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)
                }

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

                // Note, that for now, the L1->L2 transactions are free, i.e. the gasPrice
                // for such transactions is always zero, so the `refundGas` is not used anywhere
                // except for notifications for the operator for API purposes. 
                notifyAboutRefund(refundGas)

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

                let toRefundRecipient
                switch success
                case 0 {
                    // 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) {
                    mintEther(getReserved1(innerTxDataOffset), toRefundRecipient, false)
                } 

                mstore(resultPtr, success)

                debugLog("Send message to L1", success)

                // Sending the L2->L1 to notify the L1 contracts that the priority 
                // operation has been processed.
                sendToL1(true, canonicalL1TxHash, success)
            }

In essence, the function:

As I said earlier, there's no limit on the gas l1TxPreparation() is allowed to spend. As long as the bootloader doesn't run out of gas (keep in mind there are other txs inside the block), the function won't panic or revert. So the user submits an L1->L2 transaction with the maximum amount of allowed factory dependencies (32) and a gas limit so low that after the call to l1TxPreparation(), there's no gas left for the execution so that step is skipped. The last thing the bootloader does is to pay the operator (gasLimit * gasPrice), determine that there's no refund, and finally mark the tx as unsuccessful.

Tools Used

none

Recommended Mitigation Steps

After l1TxPreparation(), add a check so that it didn't consume more gas than the user provided. Or, use nearCall.

miladpiri commented 1 year ago
  1. In the audited code, the L1→L2 transactions were free, so it doesn’t matter if it was free.
  2. In the new code, the L1 contract ensures that hte user has provided enoygh gas
c4-sponsor commented 1 year ago

miladpiri marked the issue as sponsor disputed

GalloDaSballo commented 1 year ago

I have verified through the blame that TX are not free anymore.

GalloDaSballo commented 1 year ago

However the tx were free during the contest, although the comment does suggest that that was going to change: https://github.com/matter-labs/era-contracts/blame/272f11c1f7e141c6c89be3224d7a52dbbc747c0a/ethereum/contracts/zksync/facets/Mailbox.sol#L143

GalloDaSballo commented 1 year ago

At the time of submission I believe the finding was valid, I'll ask for feedback by a couple more judges but at this time am leaving the report open

c4-judge commented 1 year ago

GalloDaSballo marked the issue as selected for report

GalloDaSballo commented 1 year ago

The finding is mitigated as the current Era code charges fees, however the Warden couldn't have known that since the Era code was changed 2 weeks ago while the contest ended 3 weeks ago

miladpiri commented 1 year ago

Thanks @GalloDaSballo At the time of contest, it was assumed (as the comment states) that all the L1 -> L2 transactions were free (so any finding related to free tx that results in DoS would be invalid, because we explicitly state that).

This finding states that an attacker can register factory deps for free. While, we already mentioned that our tx are assumed to be free in the comment. So, what is the added value here?

So, I do not see this a valid finding.

GalloDaSballo commented 1 year ago

This finding states that an attacker can register factory deps for free. While, we already mentioned that our tx are assumed to be free in the comment. So, what is the added value here?

Thank you for the clarification @miladpiri , will double check

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Out of scope

GalloDaSballo commented 1 year ago

I understand that the finding pertains to Bootloader so have closed as OOS

I believe that the statement concerning DOS being disclosed does make the finding OOS in that end

That said, even with paying txs, it does seem to be like this allows DOSsing by buying all available gas in a block, I must acknowledged that this could be viewed as "normal" behaviour as ultimately at the sequencer level there's no difference between someone buying txs to use the chain or to doss it.

I recommend the warden to further elaborate on the possible attack and follow up with the Sponsor or me