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

`shouldDeployBeacon` is misinterpreted as `_contractsDeployedAlready` in constructor of `L2NativeTokenVault`

mspenceya

Summary

shouldDeployBeacon is misinterpreted as _contractsDeployedAlready in constructor of L2NativeTokenVault

Vulnerability Details

https://github.com/Cyfrin/2024-10-zksync/blob/cfc1251de29379a9548eeff1eea3c78267288356/era-contracts/system-contracts/contracts/L2GatewayUpgradeHelper.sol#L166

https://github.com/Cyfrin/2024-10-zksync/blob/cfc1251de29379a9548eeff1eea3c78267288356/era-contracts/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol#L70

While performing force deployment of NTV, a package of parameters are passed to its constructor.
The data package is provided by _getForceDeploymentsData function:

...
forceDeployments[3] = ForceDeployment({
...
input: abi.encode(
fixedForceDeploymentsData.l1ChainId,
fixedForceDeploymentsData.aliasedL1Governance,
fixedForceDeploymentsData.l2TokenProxyBytecodeHash,
additionalForceDeploymentsData.l2LegacySharedBridge,
deployedTokenBeacon,
--> shouldDeployBeacon,
wrappedBaseTokenAddress,
additionalForceDeploymentsData.baseTokenAssetId
)
});
...

However, on the receiving end (constructor of L2NativeTokenVault), it is interpreted as _contractsDeployedAlready - which is totally opposite meaning.

constructor(
uint256 _l1ChainId,
address _aliasedOwner,
bytes32 _l2TokenProxyBytecodeHash,
address _legacySharedBridge,
address _bridgedTokenBeacon,
bool _contractsDeployedAlready,
address _wethToken,
bytes32 _baseTokenAssetId
) NativeTokenVault(_wethToken, L2_ASSET_ROUTER_ADDR, _baseTokenAssetId, _l1ChainId) {
...
--> if (_contractsDeployedAlready) {
...
} else {
...
}
}

As a result, deployment operation would go in a opposite way regarding the deployment of beacon contract. Worst case, it would revert with error.

Impact

Deployment operation would go in a opposite way regarding the deployment of beacon contract. This is serious because in worst case, it would revert with error.

Tools Used

Manual Review

Mitigation Steps

Make sure the boolean value shouldDeployBeacon is correctly used in constructor of L2NativeTokenVault contract.

Updates

Lead Judging Commences

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

`L2GatewayUpgradeHelper._getForceDeploymentsData` semantic mismatch during `L2NativeTokenVault` deployment will revert if the beacon is not deployed during a `L2GenesisUpgrade.genesisUpgrade`

Support

FAQs

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