Era
ZKsync
FoundryLayer 2
500,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Inconsistent Permanent Rollup State Validation in Fee Parameter Updates

cheatc0d33

Summary

The AdminFacet contract contains an inconsistency in how it enforces permanent rollup state constraints across different fee-related functions. While setPubdataPricingMode() and makePermanentRollup() properly enforce that permanent rollups must use the Rollup pricing mode, the changeFeeParams() function lacks this validation. This creates a potential state invariant violation where a permanent rollup chain could have an invalid pricing mode.

While the current implementation of changeFeeParams() prevents direct changes to the pubdataPricingMode, it doesn't validate that the existing/incoming mode is correct for permanent rollups. This means that if a chain becomes a permanent rollup through state migration (via forwardedBridgeMint()), there's no validation ensuring the fee parameters maintain the required Rollup pricing mode.

This inconsistency could lead to:

  1. State invariant violations where a permanent rollup operates with an invalid pricing mode

  2. Potential security issues if other parts of the system assume permanent rollups always use Rollup pricing mode

  3. Difficulty in upgrading or maintaining the system due to inconsistent state validation

Proof of Concept

https://github.com/Cyfrin/2024-10-zksync/blob/cfc1251de29379a9548eeff1eea3c78267288356/era-contracts/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol#L100

The issue can be demonstrated through the following sequence of operations:

// Initial state: Chain is not a permanent rollup, using Validium mode
FeeParams memory initialParams = FeeParams({
pubdataPricingMode: PubdataPricingMode.Validium,
...
});
s.feeParams = initialParams;
s.isPermanentRollup = false;
​
// 1. Chain becomes permanent rollup through migration
ZKChainCommitment memory commitment = ZKChainCommitment({
isPermanentRollup: true,
...
});
adminFacet.forwardedBridgeMint(abi.encode(commitment), true);
// Now s.isPermanentRollup = true, but fee params still use Validium mode
​
// 2. Update fee parameters
FeeParams memory newParams = FeeParams({
pubdataPricingMode: PubdataPricingMode.Validium, // Same as old mode
...
});
// This call succeeds despite invalid state
adminFacet.changeFeeParams(newParams);

Relevant code references showing the inconsistency:

  1. setPubdataPricingMode() enforces the constraint:

if (s.isPermanentRollup && _pricingMode != PubdataPricingMode.Rollup) {
revert IncorrectPricingMode();
}

  1. changeFeeParams() lacks the same validation:

if (_newFeeParams.pubdataPricingMode != oldFeeParams.pubdataPricingMode) {
revert InvalidPubdataPricingMode();
}

Recommended mitigation steps

Add permanent rollup state validation to the changeFeeParams() function:

function changeFeeParams(FeeParams calldata _newFeeParams) external onlyAdminOrChainTypeManager onlyL1 {
if (_newFeeParams.maxPubdataPerBatch < _newFeeParams.priorityTxMaxPubdata) {
revert PriorityTxPubdataExceedsMaxPubDataPerBatch();
}
​
FeeParams memory oldFeeParams = s.feeParams;
​
// we cannot change pubdata pricing mode
if (_newFeeParams.pubdataPricingMode != oldFeeParams.pubdataPricingMode) {
revert InvalidPubdataPricingMode();
}
​
// Add validation for permanent rollup state
if (s.isPermanentRollup && _newFeeParams.pubdataPricingMode != PubdataPricingMode.Rollup) {
revert IncorrectPricingMode();
}
​
s.feeParams = _newFeeParams;
​
emit NewFeeParams(oldFeeParams, _newFeeParams);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

changeFeeParams() doesn't check that permanent rollups use the `Rollup` pricing mode

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.