If reservedGas > 0 and have paymaster, may not be able to refund reservedGas to the user.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/bootloader/bootloader.yul#L726-L731 https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/bootloader/bootloader.yul#L1425

Vulnerability details

Vulnerability details

Currently L2 supports user-specified paymaster to support custom ERC20 for eth to pay for gas.

The main steps to perform are as follows:

1. call account.prepareForPaymaster() (Usually approve token allowance to paymaster)
2. call paymaster.validateAndPayForPaymasterTransaction() to pay gas
3. bootloader receives eth that is greater than GasLimit * gasPrice, return the overpaid eth to paymaster`.
4. Execute the user transaction
5. if gasLeft > 0 execute paymaster.postTransaction(), and return the unused gasLeft + reservedGas to paymaster.

The most important thing is that paymaster.postTransaction() is not guaranteed to be called.

    /// @dev Called by the bootloader after the execution of the transaction. Please note that
    /// there is no guarantee that this method will be called at all. Unlike the original EIP4337,
@>  /// this method won't be called if the transaction execution results in out-of-gas.
    /// @param _context, the context of the execution, returned by the "validateAndPayForPaymasterTransaction" method.
    /// @param  _transaction, the users' transaction.
    /// @param _txResult, the result of the transaction execution (success or failure).
    /// @param _maxRefundedGas, the upper bound on the amout of gas that could be refunded to the paymaster.
    /// @dev The exact amount refunded depends on the gas spent by the "postOp" itself and so the developers should
    /// take that into account.
    function postTransaction(
        bytes calldata _context,
        Transaction calldata _transaction,
        bytes32 _txHash,
        bytes32 _suggestedSignedHash,
        ExecutionResult _txResult,
        uint256 _maxRefundedGas
    ) external payable;

So paymaster can't collect the corresponding ERC20 from the user in paymaster.postTransaction(), if in this step, then the user can control the remaining gas equal to 0 to skip the payment of ERC20.

So paymaster can only collect the corresponding ERC20 in the second step paymaster.validateAndPayForPaymasterTransaction().

For example the following scenario MAX_GAS_PER_TRANSACTION = 1000 Aliace performs a transaction specifying gasLimit = 1100 and paymaster, so reservedGas = 100

Based on the current logic the following steps are performed.

1. alice approve paymaster allowance = 1100
2. execute paymaster.validateAndPayForPaymasterTransaction() , paymaster transfers the 1100Token and pays the 1100ETH.
3. bootloader does not exceed gasLimit then refund 0
4. the execution of the alice transaction succeeds, but gasLeft = 0
5. because gasLeft == 0 did not execute paymaster.postTransaction(), but has returned reservedGas=100 to paymaster, paymaster can not return ERC20 to the user

Impact

Since paymaster.postTransaction() doesn't always execute, which results in not being able to refund reservedGas to the user if gasLeft==0 but reservedGas>0

Recommended Mitigation

It is still recommended that you need to ensure that paymaster.postTransaction() executes successfully. If it fails, we recommend rolling it back.

Or simply determine that if reservedGas>0, you need to run paymaster.postTransaction()

            function refundCurrentL2Transaction(
                txDataOffset,
                transactionIndex,
                success, 
                gasLeft,
                gasPrice,
                reservedGas
            ) -> finalRefund {
...
                default {
                    refundRecipient := paymaster

-                   if gt(gasLeft, 0) {
+                   if or(gt(gasLeft, 0),gt(reservedGas,0) {
                        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"),

Assessed type

Context

[c4-pre-sort]

bytes032 marked the issue as duplicate of #259

[c4-judge]

GalloDaSballo marked the issue as not a duplicate

[c4-judge]

GalloDaSballo marked the issue as duplicate of #620

[c4-judge]

GalloDaSballo marked the issue as satisfactory

[miladpiri]

The comment clearly states that it is not guaranteed to call postOp on the paymaster due to out of gas. So, it is known.
https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/interfaces/IPaymaster.sol#L34-L36

[c4-sponsor]

miladpiri (sponsor) disputed

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)