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

2 stars 1 forks source link

Permanent loss of the L1 -> L2 refund gas when creating a new chain #98

Closed c4-bot-9 closed 7 months ago

c4-bot-9 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/state-transition/StateTransitionManager.sol#L194

Vulnerability details

Impact

Setting the refundRecipient of a L1 -> L2 transaction to 0 will send the leftover gas of such a transaction to the address(0). As the hyperchain ecosystem evolves, more and more hyperchains will be created, so the amount lost over time can be considerable.

Proof of Concept

When the bridgeHub creates a new hyperchain, it will eventually call

StateTransitionManager, function createNewChain

    /// @notice called by Bridgehub when a chain registers
    function createNewChain(
        uint256 _chainId,
        address _baseToken,
        address _sharedBridge,
        address _admin,
        bytes calldata _diamondCut
    ) external onlyBridgehub { ... }

If we follow the execution path, we see that it builds from scratch the L2 transaction to be submitted in

    /// @dev we have to set the chainId at genesis, as blockhashzero is the same for all chains with the same chainId
    function _setChainIdUpgrade(uint256 _chainId, address _chainContract) internal {
        bytes memory systemContextCalldata = abi.encodeCall(ISystemContext.setChainId, (_chainId));
        uint256[] memory uintEmptyArray;
        bytes[] memory bytesEmptyArray;

        L2CanonicalTransaction memory l2ProtocolUpgradeTx = L2CanonicalTransaction({
            txType: SYSTEM_UPGRADE_L2_TX_TYPE,
            from: uint256(uint160(L2_FORCE_DEPLOYER_ADDR)),
            to: uint256(uint160(L2_SYSTEM_CONTEXT_SYSTEM_CONTRACT_ADDR)),
            gasLimit: $(PRIORITY_TX_MAX_GAS_LIMIT),
            gasPerPubdataByteLimit: REQUIRED_L2_GAS_PRICE_PER_PUBDATA,
            maxFeePerGas: uint256(0),
            maxPriorityFeePerGas: uint256(0),
            paymaster: uint256(0),
            // Note, that the priority operation id is used as "nonce" for L1->L2 transactions
            nonce: protocolVersion,
            value: 0,
            reserved: [uint256(0), 0, 0, 0],
            data: systemContextCalldata,
            signature: new bytes(0),
            factoryDeps: uintEmptyArray,
            paymasterInput: new bytes(0),
            reservedDynamic: new bytes(0)
        });

        ...
    }

and it sets all the values inside the reserved array field to 0. This is a bad decission as it is very difficult to estimate the exact amount of gas that a tx will need, so there will be some dust that must be returned to the caller. From the previous contest, the reserved[1] field denotes the refundAddress, so if it is 0, the bootloader will naively send the refund gas to the address(0), which means those funds will be permanently lost. The code snippet responsible of that is in

bootloader, lines 1029 to 1035

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

Recommended Mitigation Steps

Pass as a function argument to the createNewChain function the refundAddress for the L1 -> L2 transaction, so that the leftover gas is not lost forever:

    /// @dev we have to set the chainId at genesis, as blockhashzero is the same for all chains with the same chainId
-   function _setChainIdUpgrade(uint256 _chainId, address _chainContract) internal {
+   function _setChainIdUpgrade(uint256 _chainId, address _chainContract, address _refundAddress) internal {
        ...

        L2CanonicalTransaction memory l2ProtocolUpgradeTx = L2CanonicalTransaction({
            txType: SYSTEM_UPGRADE_L2_TX_TYPE,
            from: uint256(uint160(L2_FORCE_DEPLOYER_ADDR)),
            to: uint256(uint160(L2_SYSTEM_CONTEXT_SYSTEM_CONTRACT_ADDR)),
            gasLimit: $(PRIORITY_TX_MAX_GAS_LIMIT),
            gasPerPubdataByteLimit: REQUIRED_L2_GAS_PRICE_PER_PUBDATA,
            maxFeePerGas: uint256(0),
            maxPriorityFeePerGas: uint256(0),
            paymaster: uint256(0),
            // Note, that the priority operation id is used as "nonce" for L1->L2 transactions
            nonce: protocolVersion,
            value: 0,
-           reserved: [uint256(0), 0, 0, 0],
+           reserved: [uint256(0), _refundAddress, 0, 0],
            data: systemContextCalldata,
            signature: new bytes(0),
            factoryDeps: uintEmptyArray,
            paymasterInput: new bytes(0),
            reservedDynamic: new bytes(0)
        });

        ...
    }

    /// @notice called by Bridgehub when a chain registers
    function createNewChain(
        uint256 _chainId,
        address _baseToken,
        address _sharedBridge,
        address _admin,
-       bytes calldata _diamondCut
+       bytes calldata _diamondCut,
+       address _refundAddress
    ) external onlyBridgehub {
        ...

        // set chainId in VM
-       _setChainIdUpgrade(_chainId, stateTransitionAddress);
+       _setChainIdUpgrade(_chainId, stateTransitionAddress, _refundAddress);

        emit StateTransitionNewChain(_chainId, stateTransitionAddress);
    }

Assessed type

Other

saxenism commented 8 months ago

Invalid issue since gasPrice is 0, so operator will receive 0. Since value is also zero the refund will be 0 too.

c4-sponsor commented 8 months ago

saxenism (sponsor) disputed

alex-ppg commented 7 months ago

The Warden describes that a chain ID upgrade transaction may lose gas to the zero address, however, as the Sponsor denotes no gas expenditure will occur resulting in no gas refunds being processed. Even in a scenario whereby gas refunds would be processed, the maximum impact of this exhibit would be QA due to the minuscule amount of gas refunded to an incorrect address.

c4-judge commented 7 months ago

alex-ppg marked the issue as unsatisfactory: Invalid