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

`L2AssetRouter._ensureTokenRegisteredWithNTV` `assetId` return value is never assigned, which will cause `withdrawToken` to fail

malfurionwhitehat

Summary

The _ensureTokenRegisteredWithNTV function in the L2AssetRouter.sol contract does not assign a value to the assetId return variable. This will cause functions that rely on the assetId return value to fail, such as _withdrawSender during IAssetHandler.bridgeBurn, IAssetRouterBase.finalizeDeposit, or revert with AssetIdNotSupported.

Vulnerability Details

The _ensureTokenRegisteredWithNTV function is defined to return a bytes32 assetId. However, within the function body, assetId is never assigned a value. As a result, the function will return the default value of bytes32(0), and functions relying on it will revert

Proof of Concept

Run with yarn l1 test:foundry --match-test test_withdrawTokenFromGateway -vvvvv. The test will revert due to the issue above, when it should not.

diff --git a/era-contracts/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol b/era-contracts/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol
index 32f9307..28e6c16 100644
--- a/era-contracts/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol
+++ b/era-contracts/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol
@@ -16,6 +16,8 @@ interface IL2AssetRouter is IAssetRouterBase {
function withdraw(bytes32 _assetId, bytes calldata _transferData) external returns (bytes32);
+ function withdrawToken(address _l2NativeToken, bytes memory _assetData) external returns (bytes32);
+
function L1_ASSET_ROUTER() external view returns (address);
function withdrawLegacyBridge(address _l1Receiver, address _l2Token, uint256 _amount, address _sender) external;
diff --git a/era-contracts/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/L2GatewayTestAbstract.t.sol b/era-contracts/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/L2GatewayTestAbstract.t.sol
index e97c201..3c6dba2 100644
--- a/era-contracts/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/L2GatewayTestAbstract.t.sol
+++ b/era-contracts/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/L2GatewayTestAbstract.t.sol
@@ -65,6 +65,25 @@ abstract contract L2GatewayTestAbstract is Test, SharedL2ContractDeployer {
l2AssetRouter.withdraw(ctmAssetId, abi.encode(data));
}
+ function test_withdrawTokenFromGateway() public {
+ finalizeDeposit();
+ address newAdmin = address(0x1);
+ bytes memory newDiamondCut = abi.encode();
+ BridgehubBurnCTMAssetData memory data = BridgehubBurnCTMAssetData({
+ chainId: mintChainId,
+ ctmData: abi.encode(newAdmin, config.contracts.diamondCutData),
+ chainData: abi.encode(chainTypeManager.protocolVersion())
+ });
+ vm.prank(ownerWallet);
+ vm.mockCall(
+ address(L2_MESSENGER),
+ abi.encodeWithSelector(L2_MESSENGER.sendToL1.selector),
+ abi.encode(bytes(""))
+ );
+ address l2NativeToken = address(weth);
+ l2AssetRouter.withdrawToken(l2NativeToken, abi.encode(data));
+ }
+
function finalizeDeposit() public {
bytes memory chainData = exampleChainCommitment;
bytes memory ctmData = abi.encode(
diff --git a/era-contracts/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/_SharedL2ContractDeployer.sol b/era-contracts/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/_SharedL2ContractDeployer.sol
index b0c0ac6..10296c3 100644
--- a/era-contracts/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/_SharedL2ContractDeployer.sol
+++ b/era-contracts/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/_SharedL2ContractDeployer.sol
@@ -95,7 +95,7 @@ abstract contract SharedL2ContractDeployer is Test, DeployUtils {
beaconProxyBytecodeHash
);
- L2WrappedBaseToken weth = deployL2Weth();
+ weth = deployL2Weth();
initSystemContracts(
SystemContractsArgs({
​

Impact

L2AssetRouter.withdrawToken will revert due to an incorrect bytes32(0) assetId value.

Tools Used

Manual code review

Recommendations

Ensure that the assetId return value is properly assigned within the _ensureTokenRegisteredWithNTV function.

diff --git a/era-contracts/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol b/era-contracts/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol
index 04d92e3..2bbe9d2 100644
--- a/era-contracts/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol
+++ b/era-contracts/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol
@@ -164,6 +164,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter {
function _ensureTokenRegisteredWithNTV(address _token) internal override returns (bytes32 assetId) {
IL2NativeTokenVault nativeTokenVault = IL2NativeTokenVault(L2_NATIVE_TOKEN_VAULT_ADDR);
nativeTokenVault.ensureTokenIsRegistered(_token);
+ assetId = nativeTokenVault.assetId(_token);
}
/// @notice Initiates a withdrawal by burning funds on the contract and sending the message to L1
​
Updates

Lead Judging Commences

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

`L2AssetRouter._ensureTokenRegisteredWithNTV` `assetId` return value is never assigned, which will cause `withdrawToken` to fail

Appeal created

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

`L2AssetRouter._ensureTokenRegisteredWithNTV` `assetId` return value is never assigned, which will cause `withdrawToken` to fail

Support

FAQs

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