Gas refund to paymaster may silently fail

[c4-bot-3]

Lines of code

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

Vulnerability details

zkSync supports account abstraction natively. bootloader.yul, which is entrypoint to EraVM, will execute multiple transactions, taking some parts of the EVM on its shoulders, like refunding gas unspent on transaction to user.

When a refund for transaction is processed, function refundCurrentL2Transaction is called, which in turn uses ZKSYNC_NEAR_CALL_callPostOp() - account abstraction defined postOp (reference implementation of ERC-4337 postOp).

However, low level call result of postOp is popped, and it's the result of calling paymaster to retrieve the refund. If it fails for whatever reason, it won't be terated as a failure, and the paymaster will loose refund.

Impact

Paymaster gas refund loss

Proof of Concept

The issue starts with processL2Tx:

            function processL2Tx(
                txDataOffset,
                resultPtr,
                transactionIndex,
                gasPerPubdata
            ) {
// [...]

                let gasSpentOnExecute := 0
                let success := 0
                success, gasSpentOnExecute := l2TxExecution(txDataOffset, gasLeft, basePubdataSpent, reservedGas, gasPerPubdata)

                let refund := 0
                let gasToRefund := sub(gasLeft, gasSpentOnExecute)
                if lt(gasLeft, gasSpentOnExecute){
                    gasToRefund := 0
                }

                // Note, that we pass reservedGas from the refundGas separately as it should not be used
                // during the postOp execution.
                refund := refundCurrentL2Transaction(
                    txDataOffset,
                    transactionIndex,
                    success,
                    gasToRefund,
                    gasPrice,
                    reservedGas,
                    basePubdataSpent,
                    gasPerPubdata
                )

                notifyAboutRefund(refund)

After transaction is executed in l2TxExecution() function, gasToRefund is taken from gasLeft - gasSpentOnExecute. This value is then passed to refundCurrentL2Transaction() function.

            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()
                        // @audit result of a call to Paymaster is popped, while it might revert
                        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)
                    } 
                }

So, to summarize, the function above checks if there is paymaster defined for transaction. If yes, then it calls ZKSYNC_NEAR_CALL_callPostOp() - account abstraction defined postOp. Low level call returns success, which signifies if the call succeded of failed. This important information, because it means if the refund from paymaster to user succeded or not.

            function ZKSYNC_NEAR_CALL_callPostOp(
                abi, 
                paymaster, 
                txDataOffset, 
                txResult, 
                maxRefundedGas,
                basePubdataSpent, 
                gasPerPubdata,
                reservedGas,
            ) -> success {
// [...]
                success := call(
                    gas(),
                    paymaster,
                    0,
                    calldataPtr,
                    calldataLen,
                    0,
                    0
                )

This result is, however, popped - pop(ZKSYNC_NEAR_CALL_callPostOp[...] - meaning that the check is skipped. In case that postOp fails, it's still considered successful, however the paymaster for this transaction won't get refunded for unspent gas.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider either reverting transaction that fails on postOp, or introduce contract that would allow paymasters to retrieve failed refunds later.

Assessed type

Invalid Validation

[c4-sponsor]

saxenism (sponsor) disputed

[saxenism]

This is an invalid report, since paymaster won't lose the refund, the user will. Also, it is by design and mentioned in our doc

[alex-ppg]

The Warden specifies that a gas refund's failure is not handled and is instead ignored. I believe this behavior conforms with the intended design, as we would not want errors in gas refunds towards a paymaster to result in a transaction's failure. As such, I consider this exhibit invalid and better suited as part of an Analysis report as criticism.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid