paymaster will refund spentOnPubdata to user

[c4-bot-7]

Lines of code

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

Vulnerability details

Vulnerability details

A very important modification of this update is that the GAS spent by pubdata is collected at the final step of the transaction.

But if there is a paymaster, when executing paymaster.postTransaction(_maxRefundedGas) _maxRefundedGas does not subtract the spentOnPubdata

bootloader.yul the code is as follow：

            function refundCurrentL2Transaction(
                txDataOffset,
                transactionIndex,
                success, 
                gasLeft,
                gasPrice,
                reservedGas,
                basePubdataSpent,
                gasPerPubdata
            ) -> finalRefund {
                setTxOrigin(BOOTLOADER_FORMAL_ADDR())

                finalRefund := 0

                let innerTxDataOffset := add(txDataOffset, 32)

                let paymaster := getPaymaster(innerTxDataOffset)
                let refundRecipient := 0
                switch paymaster
                case 0 {
                    // No paymaster means that the sender should receive the refund
                    refundRecipient := getFrom(innerTxDataOffset)
                }
                default {
                    refundRecipient := paymaster

                    if gt(gasLeft, 0) {
                        checkEnoughGas(gasLeft)
                        let nearCallAbi := getNearCallABI(gasLeft)
                        let gasBeforePostOp := gas()
                        pop(ZKSYNC_NEAR_CALL_callPostOp(
                            // Maximum number of gas that the postOp could spend
                            nearCallAbi,
                            paymaster,
                            txDataOffset,
                            success,
                            // Since the paymaster will be refunded with reservedGas,
                            // it should know about it
@>                          safeAdd(gasLeft, reservedGas, "jkl"),
                            basePubdataSpent,
                            reservedGas,
                            gasPerPubdata
                        ))
                        let gasSpentByPostOp := sub(gasBeforePostOp, gas())

                        gasLeft := saturatingSub(gasLeft, gasSpentByPostOp)
                    } 
                }

                // It was expected that before this point various `isNotEnoughGasForPubdata` methods would ensure that the user 
                // has enough funds for pubdata. Now, we just subtract the leftovers from the user.
@>              let spentOnPubdata := getErgsSpentForPubdata(
                    basePubdataSpent,
                    gasPerPubdata
                )

                let totalRefund := saturatingSub(add(reservedGas, gasLeft), spentOnPubdata)

                askOperatorForRefund(
                    totalRefund,
                    spentOnPubdata,
                    gasPerPubdata
                )

                let operatorProvidedRefund := getOperatorRefundForTx(transactionIndex)

                // If the operator provides the value that is lower than the one suggested for 
                // the bootloader, we will use the one calculated by the bootloader.
                let refundInGas := max(operatorProvidedRefund, totalRefund)

                // The operator cannot refund more than the gasLimit for the transaction
                if gt(refundInGas, getGasLimit(innerTxDataOffset)) {
                    assertionError("refundInGas > gasLimit")
                }

                if iszero(validateUint32(refundInGas)) {
                    assertionError("refundInGas is not uint32")
                }

                let ethToRefund := safeMul(
                    refundInGas, 
                    gasPrice, 
                    "fdf"
                ) 

                directETHTransfer(ethToRefund, refundRecipient)

                finalRefund := refundInGas
            }

paymaster's _maxRefundedGas = gasLeft + reservedGas, without subtracting spentOnPubdata.

This way _maxRefundedGas will be much larger than the correct value

paymaster will refund the used spentOnPubdata to the user

Impact

paymaster will refund the spentOnPubdata already used by the user

Recommended Mitigation

            function refundCurrentL2Transaction(
                txDataOffset,
                transactionIndex,
                success, 
                gasLeft,
                gasPrice,
                reservedGas,
                basePubdataSpent,
                gasPerPubdata
            ) -> finalRefund {
                setTxOrigin(BOOTLOADER_FORMAL_ADDR())

                finalRefund := 0

                let innerTxDataOffset := add(txDataOffset, 32)

                let paymaster := getPaymaster(innerTxDataOffset)
                let refundRecipient := 0
                switch paymaster
                case 0 {
                    // No paymaster means that the sender should receive the refund
                    refundRecipient := getFrom(innerTxDataOffset)
                }
                default {
                    refundRecipient := paymaster
+                   let expectSpentOnPubdata := getErgsSpentForPubdata(
+                        basePubdataSpent,
+                        gasPerPubdata
+                    )                    
                    if gt(gasLeft, 0) {
                        checkEnoughGas(gasLeft)
                        let nearCallAbi := getNearCallABI(gasLeft)
                        let gasBeforePostOp := gas()
                        pop(ZKSYNC_NEAR_CALL_callPostOp(
                            // Maximum number of gas that the postOp could spend
                            nearCallAbi,
                            paymaster,
                            txDataOffset,
                            success,
                            // Since the paymaster will be refunded with reservedGas,
                            // it should know about it
-                           safeAdd(gasLeft, reservedGas, "jkl"),
+                           saturatingSub(add(reservedGas, gasLeft), expectSpentOnPubdata),
                            basePubdataSpent,
                            reservedGas,
                            gasPerPubdata
                        ))
                        let gasSpentByPostOp := sub(gasBeforePostOp, gas())

                        gasLeft := saturatingSub(gasLeft, gasSpentByPostOp)
                    } 
                }

                // It was expected that before this point various `isNotEnoughGasForPubdata` methods would ensure that the user 
                // has enough funds for pubdata. Now, we just subtract the leftovers from the user.
                let spentOnPubdata := getErgsSpentForPubdata(
                    basePubdataSpent,
                    gasPerPubdata
                )

                let totalRefund := saturatingSub(add(reservedGas, gasLeft), spentOnPubdata)

                askOperatorForRefund(
                    totalRefund,
                    spentOnPubdata,
                    gasPerPubdata
                )

                let operatorProvidedRefund := getOperatorRefundForTx(transactionIndex)

                // If the operator provides the value that is lower than the one suggested for 
                // the bootloader, we will use the one calculated by the bootloader.
                let refundInGas := max(operatorProvidedRefund, totalRefund)

                // The operator cannot refund more than the gasLimit for the transaction
                if gt(refundInGas, getGasLimit(innerTxDataOffset)) {
                    assertionError("refundInGas > gasLimit")
                }

                if iszero(validateUint32(refundInGas)) {
                    assertionError("refundInGas is not uint32")
                }

                let ethToRefund := safeMul(
                    refundInGas, 
                    gasPrice, 
                    "fdf"
                ) 

                directETHTransfer(ethToRefund, refundRecipient)

                finalRefund := refundInGas
            }

Assessed type

Context

[saxenism]

We confirm the finding. It is good.

We however believe that this is a medium severity issue since this is a rarely used functionality.

[c4-sponsor]

saxenism marked the issue as disagree with severity

[c4-sponsor]

saxenism (sponsor) confirmed

[alex-ppg]

The Warden has identified a discrepancy in the way paymaster refunds are processed for L2 transactions, resulting in an over-compensation that overlaps with the gas spent on public data.

The exhibit is correct, and I am not in complete agreement with the Sponsor's assessment in relation to the submission's severity. The referenced code will trigger if a paymaster has been defined, and I do not believe there is any constraint that permits a malicious user from always triggering the surplus refund and thus from slowly siphoning funds in the form of gas from the system.

As the flaw is always present and its impact is properly considered medium, I consider the combination of those two factors to merit a high severity rating.

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg marked the issue as selected for report