EIP-155 is not enforced, allowing attackers/malicious operators to profit from replaying transactions

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/contracts/libraries/TransactionHelper.sol#L183-L187 https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/contracts/DefaultAccount.sol#L78-L107

Vulnerability details

The L2 DefaultAccount.validateTransaction function and transaction encoding process in the bootloader do not enforce EIP-155. If the reserved[0] of the legacy tx is zero, the EIP-155 will be disabled. The chainId will not be included in the transaction signature.

        bytes memory encodedChainId;
        if (_transaction.reserved[0] != 0) {
            encodedChainId = bytes.concat(RLPEncoder.encodeUint256(block.chainid), hex"80_80");
        }

Impact

There are two potential exploitation scenarios:

1. A malicious attacker can replay transactions from the mainnet or other networks not protected by EIP-155. While there are very few EVM networks currently supporting transactions without EIP-155, attackers still have an opportunity to profit by replaying early transactions from other networks. For example, if an early ETH user A sent a legacy tx to user B before EIP-155 enabled, and now user A deposited some ETH into the Zksync era network, then user B could replay the early transaction from user A to steal his funds on the Zksync network.

2. Operators can replay early user transactions from eth/other EVM networks to collect gas fees or even profit directly.

Proof of Concept

EIP-155 will be disabled when the following two conditions are met:

1. transaction type is legacy tx, which tx_type=0 ;

2. tx.reserved[0] == 0

The value of the reserved field cannot be set arbitrarily, but if the user inputs a transaction with tx_type=0 and the v in the signature is 27 or 28, then reserved[0] will be set to 0.

Code: https://github.com/matter-labs/zksync-era/blob/6cb8c2c350385ed96c0824869a885a7285735a91/core/lib/vm/src/types/internals/transaction_data.rs#L52-L60

                let should_check_chain_id = if matches!(
                    common_data.transaction_type,
                    TransactionType::LegacyTransaction
                ) && common_data.extract_chain_id().is_some()
                {
                    U256([1, 0, 0, 0])
                } else {
                    U256::zero()
                };

    pub fn extract_chain_id(&self) -> Option<u64> {
        let bytes = self.input_data()?;
        let chain_id = match bytes.first() {
            Some(x) if *x >= 0x80 => {
                let rlp = Rlp::new(bytes);
                let v = rlp.val_at(6).ok()?;
                PackedEthSignature::unpack_v(v).ok()?.1?
            }

Tools Used

Manual review

Recommended Mitigation Steps

Enforce EIP-155 signature verification in the contract DefaultAccount.validateTransaction

Assessed type

Other

[c4-pre-sort]

bytes032 marked the issue as primary issue

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[miladpiri]

This is a design decision. So, QA.

[c4-sponsor]

miladpiri (sponsor) confirmed

[c4-sponsor]

miladpiri marked the issue as disagree with severity

[GalloDaSballo]

Downgrading to QA because this is an option, not the default

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[5z1punch]

I have privately submitted a real attack example to sponsor and judge to demonstrate that this is a critical issue. However, due to its impact on the era mainnet, I am currently not publicly disclosing them on this page. Respectfully ping @GalloDaSballo @miladpiri

[c4-judge]

This previously downgraded issue has been upgraded by GalloDaSballo

[vladbochok]

This is design decision for supporting pre EIP155 transactions, that many other EVM compatible chains supports. The exact reason for supporting pre EIP155 transaction is having benefits of keyless transactions (as deterministic deployments). Due to this reason, I don't see this as a security issue at all.

• https://yamenmerhi.medium.com/nicks-method-ethereum-keyless-execution-168a6659479c

[c4-judge]

GalloDaSballo marked the issue as selected for report

[c4-judge]

GalloDaSballo changed the severity to 2 (Med Risk)

[GalloDaSballo]

I have spent tens of hours around this issue

Investigating it's impact And investigating why this issue is considered "acceptable"

I believe that a clear divide between Developers and Security Researchers can be seen in how Severity is interpreted for this issue.

Any "EVM Developer" will look at this finding as trivial, and obvious, "it's the user fault" for having a wallet that is pre EIP-155 And the usage of this signature scheme are keyless deployments

Any SR will look at this as a means to DOS users, and potentially steal their funds

I have spoken with a lot of different actors in the security space and have heard every possible severity for this type of findings

From known to high severity

After talking with 4 different judges, am going to set the finding as Medium Severity as zkSync is not uniquely subjected to this finding, but I believe the finding to be a danger to end users which should be publicly disclosed to them