sherlock-audit / 2023-03-optimism-judging

7 stars 0 forks source link

Jeiwan - `CrossDomainMessenger` over-estimates the gas required to pass cross-chain messages and contradicts the intrinsic gas calculation, forcing users to pay more #92

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Jeiwan

medium

CrossDomainMessenger over-estimates the gas required to pass cross-chain messages and contradicts the intrinsic gas calculation, forcing users to pay more

Summary

The gas usage of cross-chain messages sent via CrossDomainMessenger.sendMessage is over-estimated: each 0 byte of data is estimated as 16 gas units, but, according to the Ethereum Yellow Paper, a zero byte of data costs 4 gas units.

Vulnerability Detail

The CrossDomainMessenger.sendMessage function is used to send cross-chain messages. The function computes the amount of gas required to pass the message to the other chain: this is done in the baseGas function, which computes the byte-wise cost of the message. The gas consumption of a message is made of three components:

  1. the dynamic overhead;
  2. the calldata overhead;
  3. and the constant minimal overhead.

The calldata overhead of a message is the amount of gas required to pass the message in a transaction. As per the Ethereum Yellow Paper (see Appendix G. Fee Schedule), passing a zero byte costs 4 gas, while passing a non-zero byte costs 16 gas. However, the baseGas function sets 16 gas per byte no matter whether the byte is zero or not.

This contradicts the intrinsic gas calculation in op-geth, which applies different gas cost to zero and non-zero bytes:

nonZeroGas := params.TxDataNonZeroGasFrontier
if isEIP2028 {
    nonZeroGas = params.TxDataNonZeroGasEIP2028
}
if (math.MaxUint64-gas)/nonZeroGas < nz {
    return 0, ErrGasUintOverflow
}
gas += nz * nonZeroGas

z := dataLen - nz
if (math.MaxUint64-gas)/params.TxDataZeroGas < z {
    return 0, ErrGasUintOverflow
}
gas += z * params.TxDataZeroGas

Thus, the gas limit paid for by users will always be bigger than the intrinsic gas of messages.

This behaviour also disagrees with how the migration process works: when computing the gas cost of legacy withdrawals, the gas cost of zero and non-zero bytes is counted correctly.

Impact

Taking into account that ABI encoding produces sparse data (many data types are left-padded with zero bytes), the effect of the over-estimation will be significant and will affect many users. Consider two most popular data types in Solidity:

  1. The address type, when ABI-encoded, is always left-padded with 12 zero bytes: out of 32 bytes of an encoded address, 12 bytes will always be zero bytes. Thus, zero bytes will make 37.5% of every address.
  2. The uint256 type, when ABI-encoded, is left-padded with zero bytes to make the encoded length of the encoded data 32 bytes. The most common use case of the type is currency amounts, which mostly use 18 decimals. Most amounts don't take the entire 32 bytes; let's assume that 12 bytes (this is ~79,228,162,514.26434e18) will be enough for the majority of amounts that are used in protocols. Thus, zero bytes will make at least 62.5% of every uint256 value.

To sum it up: 37.5% of bytes required to pass addresses and at least 62.5% of bytes required to pass uint256 values will cost 4 times more than expected. This is a significant increase of gas cost for users.

To get more practical numbers, we can check what messages the bridge contracts (the main contracts to transfer funds between L1 and L2) will pass:

  1. To bridge ETH, the following calldata is passed: 4 bytes of the finalizeBridgeETH function selector, "from" and "to" addresses, a uint256 amount of ETH to bridge, and arbitrary extra bytes. The signature (0x1635f5fd) has no zero bytes. The two addresses have 24 zero bytes. The value has 20 zero-bytes (as per our assumption above). The arbitrary can have any number of bytes. Thus, out of 76 bytes (4 + 20 + 20 + 32), 44, or ~57.8%, will be zero and will cost 4 times more gas than they should. This is a ~77% increase in gas cost ($(76 16) / ((44 4) + ((76 - 44) * 16)) = 1.7674418604651163$).
  2. To bridge ERC20, the following calldata will be passed: 4 bytes of the finalizeBridgeERC20 function selector, remote and local token addresses, "from" and "to" addresses, a uint256 amount of tokens to bridge, and arbitrary extra bytes. Out of 116 bytes (4 + 20 + 20 + 20 + 20 + 32), 68, or ~58.6% will be zero and will cost 4 times more gas than they should. This is a ~78% increase in gas cost ($(116 16) / ((68 4) + ((116 - 68) * 16)) = 1.7846153846153847$).

The two bridge contracts (L1StandardBridge and L2StandardBridge, that inherit from StandardBridge) are the main contracts on Optimism Bedrock to bridge ETH and ERC20. Thus, Optimism users will pay ~177% for the gas required to bridge tokens.

Another important moment here is how this gas is paid. When sending messages from L1 to L2, OptimismPortal meters the gas limit of each message. The metering implements an EIP-1559-like mechanism to set gas price based on the demand for gas. Due to the over-estimation of gas in CrossDomainMessenger, the mechanism will detect higher gas demand and will increase gas price faster than expected. In other words, the over-estimation will both increase gas limit and gas price–users will pay more for more gas.

Code Snippet

  1. CrossDomainMessenger.sendMessage sends cross-chain messages: https://github.com/ethereum-optimism/optimism/blob/9b9f78c6613c6ee53b93ca43c71bb74479f4b975/packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol#L247
  2. baseGas computes the amount of gas required to send a message: https://github.com/ethereum-optimism/optimism/blob/9b9f78c6613c6ee53b93ca43c71bb74479f4b975/packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol#L258
  3. baseGas assigns the price of 16 bytes to both zero and non-zero bytes: https://github.com/ethereum-optimism/optimism/blob/9b9f78c6613c6ee53b93ca43c71bb74479f4b975/packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol#L432

    Tool used

    Manual Review

    Recommendation

    In the CrossDomainMessenger.baseGas, consider assigning 4 gas to zero bytes and 16 gas to non-zero bytes, as per the Ethereum Yellow Paper. This is also how the cost of cross-chain messages is computed during the migration.

GalloDaSballo commented 1 year ago

Have suggegested QA Because I had sent this finding in the previous contest and it was not awarded as Low

GalloDaSballo commented 1 year ago

https://github.com/sherlock-audit/2023-01-optimism-judging/issues/136

Jeiwan commented 1 year ago

Escalate for 10 USDC

I believe that this finding is important to the sponsor and should be judged by the sponsor. The linked report from the previous contest (https://github.com/sherlock-audit/2023-01-optimism-judging/issues/136) was submitted with low severity and was excluded automatically (the sponsor didn't see it). Moreover, that report had very poor explanation of the impact and didn't highlight the depth and the scale of the impact.

It's important to understand that the bridging contracts (including the one in question, CrossDomainMessenger) implement the gas calculation logic of a node: both the contracts and the node estimate the gas consumption of cross-chain messages/transactions, and both of them implement EIP-1559 to discover the gas cost of cross-chain messages/transactions. It's also worth noting that cross-chain messages are converted to transactions in the Optimism node–they are, in fact, transactions, and this is why the gas cost estimation of cross-chain messages must be identical to the gas cost estimation of transactions.

However, as this report shows, the gas cost of cross-chain messages is overestimated. This creates a discrepancy between the on-chain gas estimation and the intrinsic gas estimation of the node (state_transition.go#L86-L99). As a result, all transactions created from cross-chain messages will require users to overpay for gas, while their intrinsic gas cost will be lower (the node will let them consume less gas).

I believe this finding falls under the category of "EVM equivalence vulnerabilities", which was highlighted as an important category of vulnerabilities by the sponsor (https://app.sherlock.xyz/audits/contests/63). As this finding shows, the equivalence between the official Optimism smart contracts and op-geth (which is a fork of Geth, an EVM implementation), is broken and causes all senders of cross-chain messages to overpay for the transactions. The discrepancy in gas calculation is a clear violation of the Yellow Paper.

sherlock-admin commented 1 year ago

Escalate for 10 USDC

I believe that this finding is important to the sponsor and should be judged by the sponsor. The linked report from the previous contest (https://github.com/sherlock-audit/2023-01-optimism-judging/issues/136) was submitted with low severity and was excluded automatically (the sponsor didn't see it). Moreover, that report had very poor explanation of the impact and didn't highlight the depth and the scale of the impact.

It's important to understand that the bridging contracts (including the one in question, CrossDomainMessenger) implement the gas calculation logic of a node: both the contracts and the node estimate the gas consumption of cross-chain messages/transactions, and both of them implement EIP-1559 to discover the gas cost of cross-chain messages/transactions. It's also worth noting that cross-chain messages are converted to transactions in the Optimism node–they are, in fact, transactions, and this is why the gas cost estimation of cross-chain messages must be identical to the gas cost estimation of transactions.

However, as this report shows, the gas cost of cross-chain messages is overestimated. This creates a discrepancy between the on-chain gas estimation and the intrinsic gas estimation of the node (state_transition.go#L86-L99). As a result, all transactions created from cross-chain messages will require users to overpay for gas, while their intrinsic gas cost will be lower (the node will let them consume less gas).

I believe this finding falls under the category of "EVM equivalence vulnerabilities", which was highlighted as an important category of vulnerabilities by the sponsor (https://app.sherlock.xyz/audits/contests/63). As this finding shows, the equivalence between the official Optimism smart contracts and op-geth (which is a fork of Geth, an EVM implementation), is broken and causes all senders of cross-chain messages to overpay for the transactions. The discrepancy in gas calculation is a clear violation of the Yellow Paper.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

hrishibhat commented 1 year ago

Escalation accepted

Valid low Accepting the escalation because the comment from the Sponsor was not posted here and the escalator could have had better context prior to escalating. Sponsor comment on this issue:

Gas estimation is a tradeoff for accuracy and the calculation itself consuming gas. We want to ensure there is enough gas so the naive calculation is used. This is a design decision and otherwise low as it relates to a gas optimization.

The issue in the previous contest was judged by the Sponsor and considered low.

sherlock-admin commented 1 year ago

Escalation accepted

Sponsor comment to this issue:

Gas estimation is a tradeoff for accuracy and the calculation itself consuming gas. We want to ensure there is enough gas so the naive calculation is used. This is a design decision and otherwise low as it relates to a gas optimization.

The issue in the previous contest was judged by the Sponsor and considered low.

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.