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

Users balances are deducted more than once when they deposit through the `L1ERC20Bridge::deposit` route

krisrenzo

Summary

The L1ERC20Bridge::deposit function, which enables legacy deposits, introduces a vulnerability where user balances are deducted more than once. This occurs due to misaligned assumptions between the L1ERC20Bridge and the L1AssetRouter regarding token handling. Specifically, the L1ERC20Bridge debits the user's balance and transfers the amount to its contract balance. However, the subsequent call to L1AssetRouter::depositLegacyErc20Bridge erroneously attempts to debit the user's balance again, causing multiple deductions.

Vulnerability Details

The protocol enable users to deposit through the legacy method by calling L1ERC20Bridge::deposit .
This function debits the user's deposited amount from their address, then calls L1AssetRouter::depositLegacyErc20Bridge which handle deposits from the legacy ERC20 bridge by converting them to the new bridge format and initiating the deposit through the bridgehub.

The issue is when L1ERC20Bridge::deposit is called, the depositor is first debited. The amount to deposit is withdrawn from the users balances and transferred to the L1ERC20Bridge contract balance.

/// @dev Transfers tokens from the depositor address to the native token vault address.
/// @return The difference between the contract balance before and after the transferring of funds.
function _approveFundsToAssetRouter(address _from, IERC20 _token, uint256 _amount) internal returns (uint256) {
uint256 balanceBefore = _token.balanceOf(address(this));
@> _token.safeTransferFrom(_from, address(this), _amount);
LN230@> bool success = _token.approve(address(L1_ASSET_ROUTER), _amount);
if (!success) {
revert ApprovalFailed();
}
uint256 balanceAfter = _token.balanceOf(address(this));
​
return balanceAfter - balanceBefore;
}

Note that the L1ERC20Bridge contract anticipates the L1AssetRouter pulling these tokens from its balance, as evident by the approval line 230. Additionally, in L1AssetRouter::depositLegacyErc20Bridge, the asset router gives approval to the NativeTokenVault (the contract responsible for handling bridged assets) to move these tokens from its balance.

The issues:

Note that there are two issues (root causes) here

While L1AssetRouter assumes the tokens have already been sent to it balance, L1ERC20Bridge was also assuming depositLegacyErc20Bridge will pull the tokens from its balance, leading to a situation where the tokens remained in L1ERC20Bridge balance -- beyond the reach of NativeTokenVault.
Secondly, when _burn was called in L1AssetRouter::depositLegacyErc20Bridge, the original depositor address is passed in as the _originalCaller, instead of the contract actually holding the tokens to be burned. This lead to the users balance been debited again when the NativeTokenVault::bridgeBurn is finally called. The L1ERC20Bridge shouldn't have debited the user in the first place, they were still going to end up debited again down the call trace.

/// @inheritdoc IAssetHandler
/// @notice Allows bridgehub to acquire mintValue for L1->L2 transactions.
/// @dev In case of native token vault _data is the tuple of _depositAmount and _receiver.
function bridgeBurn(
uint256 _chainId,
uint256,
bytes32 _assetId,
address _originalCaller,
bytes calldata _data
) external payable override onlyAssetRouter whenNotPaused returns (bytes memory _bridgeMintData) {
if (originChainId[_assetId] != block.chainid) {
@> _bridgeMintData = _bridgeBurnBridgedToken(_chainId, _assetId, _originalCaller, _data);
} else {
_bridgeMintData = _bridgeBurnNativeToken({
_chainId: _chainId,
_assetId: _assetId,
_originalCaller: _originalCaller,
_depositChecked: false,
_data: _data
});
}
}
​
​

Impact

The vulnerability results in users being debited multiple times for the same deposit, leading to an incorrect reduction in their balances. This creates financial losses for users and undermines the trust in the system.

Tools Used

Manual review

Recommendations

Align Token Handling Assumptions: Ensure L1ERC20Bridge and L1AssetRouter have consistent assumptions about where tokens reside during the deposit process.

Updates

Lead Judging Commences

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

Double spending of funds when bridging “bridgedToken”

Support

FAQs

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