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

`TokensWithFeesNotSupported` can by bypassed resulting in potential protocol loss in `NativeTokenVault`

bladesec

Summary

Fee-on-transfer tokens (e.g., STA, PAXG) can be transferred through the L1AssetRouter.transferFundsToNTV function if specific allowances are set, bypassing the TokensWithFeesNotSupported check in the NativeTokenVault._bridgeBurnNativeToken function. Consequently, the NativeTokenVault will retain less than the amount required to be minted on L2, leading to a mismatch and potential protocol loss.

Vulnerability Details

Bridging action can start at Bridgehub.requestL2TransactionDirect, which consequently calls L1AssetRouter.bridgehubDepositBaseToken and then NativeTokenVault.bridgeBurn().
NativeTokenVault.bridgeBurn() calls internal _bridgeBurnNativeToken function to carry out the deposit action.

From here, there are 2 flows that user can transfer his token to NTV:

  • via L1AssetRouter.transferFundsToNTV when the _originalCaller allowed L1AssetRouter of amount, but less for nativeTokenVault

  • via NativeTokenVault._depositFunds otherwise

However, transferFundsToNTV does not verify the actual transferred amount after the transfer, unlike in the other flow where _depositFunds is used.
Below are the implementation of transferFundsToNTV and _bridgeBurnNativeToken functions:

/// @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) {
// slither-disable-next-line arbitrary-send-erc20
>> l1Token.safeTransferFrom(_originalCaller, address(nativeTokenVault), _amount);
return true;
}
return false;
}

As outlined above, l1Token is just transffered from _originalCaller to address(nativeTokenVault) but no additional check is conducted afterwards. Contrarily, there's a check on expectedDepositAmount when _depositFunds is used:

function _bridgeBurnNativeToken(
uint256 _chainId,
bytes32 _assetId,
address _originalCaller,
bool _depositChecked,
bytes calldata _data
) internal virtual returns (bytes memory _bridgeMintData) {
(uint256 _depositAmount, address _receiver) = abi.decode(_data, (uint256, address));
​
uint256 amount;
address nativeToken = tokenAddress[_assetId];
if (_assetId == BASE_TOKEN_ASSET_ID) {
amount = msg.value;
​
// In the old SDK/contracts the user had to always provide `0` as the deposit amount for ETH token, while
// ultimately the provided `msg.value` was used as the deposit amount. This check is needed for backwards compatibility.
if (_depositAmount == 0) {
_depositAmount = amount;
}
_handleChainBalanceIncrease(_chainId, _assetId, amount, true);
if (_depositAmount != amount) {
revert ValueMismatch(_depositAmount, amount);
}
} else {
// The Bridgehub also checks this, but we want to be sure
if (msg.value != 0) {
revert NonEmptyMsgValue();
}
amount = _depositAmount;
_handleChainBalanceIncrease(_chainId, _assetId, amount, true);
if (!_depositChecked) {
>> uint256 expectedDepositAmount = _depositFunds(_originalCaller, IERC20(nativeToken), _depositAmount); // note if _originalCaller is this contract, this will return 0. This does not happen.
// The token has non-standard transfer logic
>> if (amount != expectedDepositAmount) {
revert TokensWithFeesNotSupported();
}
}
}
...
}

By design, Fee-on-Transfer (FoT) tokens are not supported to ensure 1:1 backing in bridge operations. However, the transferFundsToNTV function allows token transfers to complete while bypassing the amount != expectedDepositAmount check, enabling FoT tokens to be improperly bridged.

Impact

The TokensWithFeesNotSupported validation can be bypassed, leading to potential protocol loss in NativeTokenVault. Fee-on-transfer tokens (e.g., STA, PAXG) can still be transferred via the L1AssetRouter.transferFundsToNTV function if specific allowances are made. This bypasses the TokensWithFeesNotSupported check in the NativeTokenVault._bridgeBurnNativeToken function, resulting in NativeTokenVault holding less than the required amount to be minted on L2, causing a discrepancy and potential loss.

Tools Used

Manual Review

Recommendations

Conduct the additional check amount != expectedDepositAmount after l1Token is transferred in transferFundsToNTV:

/// @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) {
// slither-disable-next-line arbitrary-send-erc20
+ uint256 balanceBefore = l1Token.balanceOf(address(nativeTokenVault));
l1Token.safeTransferFrom(_originalCaller, address(nativeTokenVault), _amount);
+ uint256 balanceAfter = l1Token.balanceOf(address(nativeTokenVault));
+ uint256 expectedDepositAmount = balanceAfter - balanceBefore;
+ if (_amount != expectedDepositAmount) {
+ revert TokensWithFeesNotSupported(); // should import error
+ }
return true;
}
return false;
}
Updates

Lead Judging Commences

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.