c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/bootloader/bootloader.yul#L913-L925 https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/bootloader/bootloader.yul#L1449-L1456 https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/system-contracts/bootloader/bootloader.yul#L1125-L1147

Vulnerability details

Impact

the impact of this issue can be one of these two:

bootloader will send more refund gas to L1 transactions which will fund loss for operator.
bootloader will send less refund gas to L2 transactions which will be fund loss for users.

I believe this issue has High severity because:

Issue will happen to all transactions (either L1 or L2) that have gas > MAX_GAS_LIMIT.
in current design, system doesn't calculates required gas upfront correctly and it depends on that fact that the unused gas is going to be refunded for user, but the issue will undermine that fact.
even so the loss value (in ETH) for each is low but because issue will happen all the time, overtime the loss can be very high.
this will be inconvenient for all the users.
the amount of extra gas that L1 users will receive can be up to 48M gas.
the amount of gas that L2 users will not receive can be up to 80M gas.

Proof of Concept

function refundCurrentL2Transaction() in boot loader is responsible for refunding unused gas to L2 transaction's owner. function processL1Tx() in bootloader is responsible for refunding unused gas to L1 transaction's owner. now let's compare the logics for refund gas in these function that handles L1 and L2 transactions:

                This is for L1 transaction:
                refundGas := max(getOperatorRefundForTx(transactionIndex), potentialRefund) // potentialRefund == gasLeft
                refundGas := add(refundGas, reservedGas)

                // this is for L2 transactions:
                let operatorProvidedRefund := getOperatorRefundForTx(transactionIndex)
                let refundInGas := max(operatorProvidedRefund, add(reservedGas, gasLeft))

in clear syntax those logics are: (reservedGas is not zero)

                //This is for L1 transaction:
                L1refundGas = max(operatorProvided, gasLeft) + reservedGas

                // this is for L2 transactions:
                L2refundInGas = max(operatorProvided, gasLeft + reservedGas)

so it's obvious that the the expression is different. there is two possibility here: (from code and docs it is not obvious that which of them is true, but based on operator's and node's code, both of them can be true (not in the same time)):

if operatorProvidedRefun is going to be the trusted-refund(refund related to TxTrustedGas) then the L1 calculation is right.
if operatorProvidedRefun is going to be the total-refund(refund related to TxTotalGasLimit) then the L2 calculation is right

in case (1) is true, then for L2 transactions, bootloader send less refund gas to L2 transaction's owner, the amount of less refund can be up to TxTrustedGas so it can be very high.

in case (2) is true, then for L1 transactions code refund more gas to L1 transaction's owner, amount of extra refund can be up to reservedGas and because L1 transaction's reservedGas can be up to 48M gas so the extra refund is very much.

in both scenarios we assumed operator is honest one and will suggest the correct overhead for each transaction. and the calculations of the loss was based on correct valid overhead. so overall this inconsistency will cause operator or users to lose funds and it will happen always for transactions with gas limit higher than 80M.

Tools Used

VIM

Recommended Mitigation Steps

fix one of the formulas based on definition of the OperatorRefundForTx.

note:

I don't have backstage access and I can't comment during Post-Judging QA duration. so if you as judge or sponsor have any question regarding the issue please contact me directly.

I reported another issue which is "operator can bypass checks and refund no gas to L1 transactions" too, that report is not the same is this one.

Assessed type

Math

c4-pre-sort commented 1 year ago

141345 marked the issue as primary issue

c4-pre-sort commented 1 year ago

141345 marked the issue as sufficient quality report

miladpiri commented 1 year ago

The L1 refund calculation is slightly off. So, Low severity.

c4-sponsor commented 1 year ago

miladpiri marked the issue as disagree with severity

c4-sponsor commented 1 year ago

miladpiri (sponsor) confirmed

c4-judge commented 11 months ago

GalloDaSballo changed the severity to QA (Quality Assurance)

0xunforgiven commented 11 months ago

Hi @GalloDaSballo, @miladpiri. Thanks for reviewing my comment.

Respectfully I believe sponsor claims regarding "The L1 refund calculation is slightly off" is wrong. While it's true that L1->L2 tx's refund over calculated(higher than real value), The difference can be big. It can be verified by comparing two math formula that L1 calculation can be higher than L2 by margin reservedGas. I humbly ask judge to check these two formulas:

                //This is for L1 transaction:
                L1refundGas = max(operatorProvided, gasLeft) + reservedGas

                // this is for L2 transactions:
                L2refundGas = max(operatorProvided, gasLeft + reservedGas)

Suppose a tx have:

gasLimit = 110M
operator set trusted gas as 80M so reservedGas = 110M-80M = 30M
suppose tx finishes when gasLeft = 50M.
and operatorProvided = 80M (operator suggest refund as sum of 50M remain gas and 30M reserved gas)
now let's compare this two formula:
```
         L1refundGas = max(operatorProvided, gasLeft) + reservedGas = max(80M, 50M) + 30M = 110M
         L2refundGas = max(operatorProvided, gasLeft + reservedGas) = max(80M , 50M+30M) = 80M
```
as you can see the L1 calculation difference is 110M - 80M = 30M and it's not a trivial amount. so this proves that extra refund gas is big(as I mentioned in the report, the loss for operator can be up to 48M gas cost). the fund loss for operator is not trivial and attacker can trigger the bug permissionless by repeating the same L1->L2 tx. The more important point is that because bootloader overcalculates refund gas, this can result in: overhead > gasLimit for tx, which has very critical Impact.

In fact, The existence of this vulnerability(over calculating L1->L2 tx's overhead) permits the creation of L1->L2 transactions in Mailbox contract that can trigger a revert in the Bootloader during processing. Consequently, an attacker can potentially halt the processing of L1->L2 transactions. This vulnerability stems from the Bootloader's wrong overhead calculation, which utilizes the formula max(operatorProvided, gasLeft) + reservedGas. This calculated overhead can exceed the transaction's gasLimit, and a code check in the Bootloader triggers a revert in such instances. Since the operator is unaware of this issue, they provide the actual refund amount. However, the Bootloader adds the additional reservedGas, causing the transaction's overhead to surpass the transaction's gasLimit.

In summary, an attacker can maliciously introduce an L1->L2 transaction into the Mailbox's queue, resulting in a calculated overhead exceeding the transaction's gas limit. The Bootloader's embedded checks will subsequently trigger a revert, enabling the attacker to halt zkSync's processing of L1->L2 transactions without requiring any permissions.

In the following section I will provide POC why this issue can be used to:

DOS the L1->L2 bridge with nearly zero cost.
Halt L1->L2 bridge without any permission required.

Based on the these facts, And the fact that sponsor confirm the overcalculation, I believe that this issue has Medium probability and High impact. So suggest High severity for the issue.

DOS:

In the above example for L1->L2 tx the gasLimit = 110M and the calculated refundGas = 110M. this means that anyone could use this and send L1->l2 txs that are free in L2 (users pays 110M gas fee but receive 110M gas fee in the end, of course in practice the numbers are not gonna be round and attacker need to binary search to find the values).

Attacker can use this and flood L1->L2 bridge with these transactions and effectively DOS the bridge and zkSync. the cost of the attack is near zero.

Halting zkSync Bridge:

Operator is gonna report providedRefund = gasLimit - consumedGas, to make the issue happen as Halt, attacker only need to make sure reservedGas > consumedGas, then providedRefund + resevedGas > gasLimit. the amount of reservedGas and refundGas and upFrontOverhead can be effected by operator suggestions but operator algorithm(zkSync node) is deterministic and attacker can find right values for his tx by simulating the tx locally.

This is POC for attacker's L1->L2 tx:

gasLimit = 128M
operator set trusted gas as 80M, then reservedGas = 48M
the upFrontOverhead will be about 48.2M gas. (equal to the whole batch overhead)
suppose tx reverts after using 1M gas, so leftGas = 80M - 48.2M - 1M = 30.8M),
because tx reverted early, so operator algorithm will refund most of the upFrontOverhead in refund calculation. suppose operator calculates realOverHeadGas = 2M
then operator is gonna provide refund gas as operatorProvided = reserveGas + leftGas + upFrontOverhead - realOverHeadGas = 48M + 30.8M + 48.2M - 2M = 125M (the algorithm for operator provided refund gas is not known but it's mentioned that it's going to be the real refund amount, which is calculated here)
now the calculated overhead in bootload will be L1Overhead = max(operatorProvided, gasLeft) + reservedGas = max(125M, 30.8M) + 48M = 173M
as you can see 173M > 128M which means calculated L1TxOverhead is bigger than tx's gasLimit and because of this check in the bootloader, the whole bootloader will revert.

regarding a tx having more than 80M gas:

The documentation clearly acknowledges the possibility of transactions exceeding 80M gas, stating: proofs that this issue is possible:

in case the users may need to use a lot of pubdata (for instance to publish the bytecode of a new contract), the required gasLimit may go way beyond the MAX_TRANSACTION_GAS_LIMIT (since the contracts can be 10s of kilobytes in size).

This indicates that transactions surpassing 80M gas are not only possible but also anticipated for specific use cases. These transactions, though infrequent, are crucial for users and projects.

For L1->L2 transactions, the gas limit in question refers to the calldata gas specified in the call to the Mailbox contract, not the L1 transaction gas. Anyone can call Mailbox.requestL2Transaction(target, l2Value, callData, 100M gas, ....) to initiate an L2 transaction with 100M gas.
The 80M limit mentioned earlier applies to the transaction body's gas(txBodyGas), not the gasLimit. The code subtracts overhead gas from gasLimit to determine the remaining gas (txBodyGas), which should be less than 80M. judge can confirm this by this lines in code: link1 link2

GalloDaSballo commented 11 months ago

Post Judging QA is not meant to add additional info, but to provide context for the information in the original report

The original report states that an incorrect refund will be calculated for transactions that are above 80M gas in consumption

To which we'd have to determine the likelihood of said transactions

From scouting the chain, and having done a similar exercise for another L2, I am very confident that most transactions do not go above 3 MLN in gas, very few go above that

I have also expressed the judgement of broadcasting transactions close or above the gas limit as QA as they require a irrealistic amount of gas consumed

For this reason, given the contents of the original report, I believe QA is most appropriate

code-423n4 / 2023-10-zksync-findings

Fund loss for users or operator because the calculation for refundGas is not consistent for L1 and L2 transactions #931