Smart contract using the bridge and mailbox is very likely to lose fund

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/contracts/ethereum/contracts/zksync/facets/Mailbox.sol#L248 https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/contracts/ethereum/contracts/bridge/L1ERC20Bridge.sol#L196 https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/bootloader/bootloader.yul#L1656

Vulnerability details

Impact

Smart contract using the bridge and mailbox is very likely to lose fund

Proof of Concept

first, Address alias should not be applied to refundRecipient

Other L2 solution such as optimism or arbitrum implement adderss aliasing as well

https://docs.arbitrum.io/arbos/l1-to-l2-messaging#address-aliasing

The Arbitrum protocol's usage of L2 Aliases for L1-to-L2 messages prevents cross-chain exploits that would otherwise be possible if we simply reused the same L1 addresses as the L2 sender; i.e., tricking an L2 contract that expects a call from a given contract address by sending retryable ticket from the expected contract address on L1.

and in the current zksync implementation, address aliasing correctly applies to msg.sender on l1 mailbox facet when requesting l2 transaction

However, the address aliasing is applied to the refundRecipient as well, and the adddress alias is applied twice,

the first time it is in the L1ERC20Bridge.sol / L1WETHBridge.sol if the refund recipient is not set, address alias applies to refund recipient address

if the user set the refund address, address aliasing still applied when the transaction is requested from the mailbox facet contract

// If the `_refundRecipient` is not provided, we use the `_sender` as the recipient.
address refundRecipient = _refundRecipient == address(0) ? _sender : _refundRecipient;
// If the `_refundRecipient` is a smart contract, we apply the L1 to L2 alias to prevent foot guns.
if (refundRecipient.code.length > 0) {
    refundRecipient = AddressAliasHelper.applyL1ToL2Alias(refundRecipient);
}

in the case of a smart contract wants to receive the refund, the refund is lost

suppose a multisig wallet request ERC20 deposit bridge and set the refund recipient as the mutlsig wallet on l2 zk rea, the multisig wallet address is the same for both ethereum and l2 zk sync era,

but in this case the address aliasing still applies to the refundRecipient, the owner can hold the multisig address on zk sync era, but he does not hold the address that multisig address + 0x1111000000000000000000000000000000001111, so the refund is lost

second, user can request transaction that executes on l2 via mailbox

if the msg.sender is a smart contract, the address aliasing applies to the msg.sender address in this line of code in mailbox contract

then the params.sender is set

this sender is then serialized as the "address from"

  ) internal pure returns (L2CanonicalTransaction memory transaction) {
        transaction = L2CanonicalTransaction({
            txType: PRIORITY_OPERATION_L2_TX_TYPE,
            from: uint256(uint160(_priorityOpParams.sender)),

the problem is that the ETH is minted to this already aliased address "from" when bootloader process the L1 transaction

setGasPrice(gasPrice)

debugLog("execution itself", 0)

let value := getValue(innerTxDataOffset)
if value {
    mintEther(from, value, true)
}

clearly it is difficult for the owner to control both smart contract address in l1 and the aliased address in l2

but the ETH is minted to aliased address in l2, cause loss of fund

again, to summarize, I think the point I really want to make is:

can apply address aliasing to msg.sender to not let the same msg.sender trigger transaction on l2 because using msg.sender for authorization is so common

but should not apply address aliasing to fund recipient, otherwise, Smart contract using the bridge and mailbox is very likely to lose fund

Tools Used

Manual Review

Recommended Mitigation Steps

it is ok to applies address aliasing to msg.sender when requesting l2 transaction, but there is no need to apply address aliasing for refundRecipient

and shoud let user specify an address to receive the minted ETH instead of minting the ETH to aliased address on l2

Assessed type

DoS

[c4-pre-sort]

bytes032 marked the issue as duplicate of #827

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[JeffCX]

https://github.com/code-423n4/2023-10-zksync/blob/main/docs/Smart%20contract%20Section/zkSync%20fee%20model.md#refunds

from the docs

it is very common for user to overpay the transaction and the user expects to receive the overpaid fund as refund

but because the address alias applies twice

the refund is lost

if the user expliclty specify the recipient address and that recipient address is a smart contract in l1, there is no need to apply alias again, otherwise, the refund is lost

so I politely think because refund is very common use case for protocol and the pre-condition is user set smart contract on l1 as refund recipient that cause loss of fund

Please consider bumping the severity to medium instead of QA

issue https://github.com/code-423n4/2023-10-zksync-findings/issues/794 summarize the problem well as well

thanks

I respect judge's final decision!

[JeffCX]

https://github.com/code-423n4/2023-10-zksync-findings/issues/913 is a good duplicate as well

I mean, it would be possible a multisig address want to bridge ETH and does not set recipient address and then all refund is lost

[c4-judge]

This previously downgraded issue has been upgraded by GalloDaSballo

[c4-judge]

GalloDaSballo marked the issue as satisfactory

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)