c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L3645-L3651 https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L12-L32 https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L3647 https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L3651

Vulnerability details

Impact

The operator provides the L1 gas price and fair L2 gas price. These are not verified on-chain and could be manipulated to overcharge users. The contract does have sanity check limits, but they could still be abused.

If the operator did manipulate the prices, total fees paid by users would be higher than expected. Historical fee data could be analyzed to estimate potential impact.

Proof of Concept

Here is why the sanity check limits could still be abused. The unverified L1 gas price and fair L2 gas price are provided by the operator here #L3647-3651

/// @notice The gas price on L1 for ETH. In the future, a trustless value will be enforced.  
/// For now, this value is trusted to be fairly provided by the operator.
let L1_GAS_PRICE := mload(128)

/// @notice The minimal gas price that the operator agrees upon.
/// In the future, it will have an EIP1559-like lower bound.  
let FAIR_L2_GAS_PRICE := mload(160)

The sanity check limits are here #L12-32

function MAX_ALLOWED_L1_GAS_PRICE() -> ret {
  // 100k gwei
  ret := 100000000000000 
}

function MAX_ALLOWED_FAIR_L2_GAS_PRICE() -> ret {
   // 10k gwei
   ret := 10000000000000
}

function validateOperatorProvidedPrices(l1GasPrice, fairL2GasPrice) {
  if gt(l1GasPrice, MAX_ALLOWED_L1_GAS_PRICE()) {
     assertionError("L1 gas price too high")
  }

  if gt(fairL2GasPrice, MAX_ALLOWED_FAIR_L2_GAS_PRICE()) {
     assertionError("L2 fair gas price too high")
  }
}

So while there are sanity checks on the max limits, the operator still has control to set the prices within those limits.

Here is an explanation of the potential issue with the operator providing unverified L1 and L2 gas prices.

The L1 gas price refers to the gas price on the Ethereum mainnet. It is needed to calculate fees for data published by the L2.
The fair L2 gas price is the minimum gas price the operator agrees to for L2 transactions.

In the Bootloader contract, these values are provided by the operator via the following code: #L3647, 3651

let L1_GAS_PRICE := mload(128) 
let FAIR_L2_GAS_PRICE := mload(160)

The values are loaded from specified memory slots. This implies the operator sets these values externally before calling the Bootloader.

Potential issue

Since these values are not verified on-chain, a malicious operator could manipulate them.
For example, they could set very high L1/L2 gas prices to overcharge users on fees.

The contract tries to limit manipulation by enforcing sanity check limits

function MAX_ALLOWED_L1_GAS_PRICE() -> ret {
  // 100k gwei 
  ret := 100000000000000  
}

function MAX_ALLOWED_FAIR_L2_GAS_PRICE() -> ret {
  // 10k gwei
  ret := 10000000000000
}

function validateOperatorProvidedPrices(l1GasPrice, fairL2GasPrice) {
  // Revert if prices exceed limits
  if (l1GasPrice > MAX_L1_GAS_PRICE) {
     assertionError("L1 gas price too high")
  }

  if (fairL2GasPrice > MAX_L2_GAS_PRICE) {  
     assertionError("L2 fair gas price too high")
  }
}

However, the operator can still set the prices quite high within these limits to overcharge users. Relying on unverified operator provided inputs for fee calculations leaves the contract vulnerable to manipulation.

Tools Used

Manual review

Recommended Mitigation Steps

Use a decentralized oracle to provide verified gas price values
Tighten the sanity check limits
Additional logic to ensure the provided prices are reasonably close to external reference prices

Assessed type

Governance

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #764

c4-judge commented 1 year ago

GalloDaSballo marked the issue as not a duplicate

GalloDaSballo commented 1 year ago

It's the one honest assumption