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

Inconsistent Token Transfer Handling for Non-Standard ERC20 Tokens in `L1AssetRouter:: transferFundsToNTV`

siisivan

Summary

The L1AssetRouter:: transferFundsToNTV function assumes that the _amount of tokens specified will be successfully transferred from the user or legacyBridge to the NativeTokenVault. However, this assumption does not hold for non-standard ERC20 tokens, such as fee-on-transfer tokens or rebalancing tokens, which can result in an incorrect amount being transferred. This may lead to discrepancies in L1 and L2 states, and in some cases, funds may become locked in the NativeTokenVault.

Vulnerability Details

When users call Bridgehub::requestL2TransactionDirect/requestL2TransactionTwoBridges on L1 to perform an L1->L2 transaction, they must deposit token to L1NativeTokenVault to mint a corresponding amount of tokens on L2 at a 1:1 ratio. The deposit logic is implemented in L1AssetRouter::transferFundsToNTV, which uses safeTransferFrom to transfer tokens from _originalCaller ( or legacyBridge ) to L1NativeTokenVault. The code is as follows:

/// @inheritdoc IL1AssetRouter
function transferFundsToNTV(
bytes32 _assetId,
uint256 _amount,
address _originalCaller
) external onlyNativeTokenVault returns (bool) {
address l1TokenAddress = INativeTokenVault(address(nativeTokenVault)).tokenAddress(_assetId);
if (l1TokenAddress == address(0) || l1TokenAddress == ETH_TOKEN_ADDRESS) {
return false;
}
IERC20 l1Token = IERC20(l1TokenAddress);
​
// Do the transfer if allowance to Shared bridge is bigger than amount
// And if there is not enough allowance for the NTV
bool weCanTransfer = false;
if (l1Token.allowance(address(legacyBridge), address(this)) >= _amount) {
_originalCaller = address(legacyBridge);
weCanTransfer = true;
} else if (
l1Token.allowance(_originalCaller, address(this)) >= _amount &&
l1Token.allowance(_originalCaller, address(nativeTokenVault)) < _amount
) {
weCanTransfer = true;
}
if (weCanTransfer) {
l1Token.safeTransferFrom(_originalCaller, address(nativeTokenVault), _amount);
return true;
}
return false;
}

However, the actual transferred amount is not validated against _amount, which poses a significant risk for non-standard tokens, such as:

Fee-on-transfer tokens: Actual transferred amounts may be less than _amount.
Rebalancing tokens: Balances may adjust dynamically, leading to discrepancies with _amount.

This could result in inconsistencies between the L1 and L2 states. For example:

Tokens minted on L2 are based on _amount, but the actual amount received by L1NativeTokenVault may be less.
Recovery of failed deposits (bridgeRecoverFailedTransfer) on L2 may fail due to insufficient L1 funds. L2->L1 withdrawals could also fail, locking funds in the L1 vault.

Impact

Inconsistent states between L1 and L2: Over-minting on L2 could lead to user balance discrepancies and loss of trust.
Funds locked in L1NativeTokenVault: Insufficient funds could block operations such as: Failed deposit recovery (bridgeRecoverFailedTransfer); L2->L1 withdrawals. Ultimately, funds may become permanently inaccessible.

Tools Used

Manual

Recommendations

After invoking safeTransferFrom, verify the actual amount transferred by comparing the L1NativeTokenVault balance before and after the transaction. Improved code:

+ error TokensWithFeesNotSupported();
/// @inheritdoc IL1AssetRouter
function transferFundsToNTV(
bytes32 _assetId,
uint256 _amount,
address _originalCaller
) external onlyNativeTokenVault returns (bool) {
address l1TokenAddress = INativeTokenVault(address(nativeTokenVault)).tokenAddress(_assetId);
if (l1TokenAddress == address(0) || l1TokenAddress == ETH_TOKEN_ADDRESS) {
return false;
}
IERC20 l1Token = IERC20(l1TokenAddress);
​
// Do the transfer if allowance to Shared bridge is bigger than amount
// And if there is not enough allowance for the NTV
bool weCanTransfer = false;
if (l1Token.allowance(address(legacyBridge), address(this)) >= _amount) {
_originalCaller = address(legacyBridge);
weCanTransfer = true;
} else if (
l1Token.allowance(_originalCaller, address(this)) >= _amount &&
l1Token.allowance(_originalCaller, address(nativeTokenVault)) < _amount
) {
weCanTransfer = true;
}
if (weCanTransfer) {
+ uint256 balanceBefore = l1Token.balanceOf(address(nativeTokenVault));
l1Token.safeTransferFrom(_originalCaller, address(nativeTokenVault), _amount);
+ uint256 balanceAfter =l1Token.balanceOf(address(nativeTokenVault));
+ if(balanceAfter - balanceBefore != _amount) {
+ revert TokensWithFeesNotSupported();}
return true;
}
return false;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
7 months ago
inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[INVALID] Inconsistent Token Transfer Handling for Non-Standard ERC20 Tokens in `L1AssetRouter:: transferFundsToNTV`

Appeal created

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

Checks in `_depositFunds` can be bypassed, causing tokens to not be backed 1 to 1 in the bridge

Support

FAQs

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