Era

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

Double spending of funds when bridging “bridgedToken”

Summary

When a user bridges a BridgedToken, the user will spend twice their funds.

Vulnerability Details

When a user bridges the BridgedToken to L2, they need to deposit their funds into the L1ERC20Bridge contract. So, when the user calls L1ERC20Bridge::deposit, it triggers the _approveFundsToAssetRouter function, which executes the fund transfer from the user to the L1ERC20Bridge contract. Here's how the code works:

function deposit(
address _l2Receiver,
address _l1Token,
uint256 _amount,
uint256 _l2TxGasLimit,
uint256 _l2TxGasPerPubdataByte,
address _refundRecipient
) public payable nonReentrant returns (bytes32 l2TxHash) {
if (_amount == 0) {
// empty deposit amount
revert EmptyDeposit();
}
if (_l1Token == ETH_TOKEN_ADDRESS) {
revert ETHDepositNotSupported();
}
=> uint256 amount = _approveFundsToAssetRouter(msg.sender, IERC20(_l1Token), _amount);
function _approveFundsToAssetRouter(address _from, IERC20 _token, uint256 _amount) internal returns (uint256) {
uint256 balanceBefore = _token.balanceOf(address(this));
=> _token.safeTransferFrom(_from, address(this), _amount);
bool success = _token.approve(address(L1_ASSET_ROUTER), _amount);
if (!success) {
revert ApprovalFailed();
}
uint256 balanceAfter = _token.balanceOf(address(this));
return balanceAfter - balanceBefore;
}

After that, the L1ERC20Bridge::deposit triggers the L1AssetRouter::depositLegacyErc20Bridge function, passing _originalCaller as msg.sender, which is the address of the user interacting with the L1ERC20Bridge::deposit function. This _originalCaller parameter is forwarded to L1AssetRouter::depositLegacyErc20Bridge:
Look at this code depositLegacyErc20Bridge

#L1ERC20Bridge.sol
function deposit(
/////////////////////////////
=> l2TxHash = L1_ASSET_ROUTER.depositLegacyErc20Bridge{value: msg.value}({
=> _originalCaller: msg.sender,
_l2Receiver: _l2Receiver,
_l1Token: _l1Token,
_amount: _amount,
_l2TxGasLimit: _l2TxGasLimit,
_l2TxGasPerPubdataByte: _l2TxGasPerPubdataByte,
_refundRecipient: _refundRecipient
});
# L1AssetRouter.sol
function depositLegacyErc20Bridge(
=> address _originalCaller,
address _l2Receiver,
address _l1Token,
uint256 _amount,
uint256 _l2TxGasLimit,
uint256 _l2TxGasPerPubdataByte,
address _refundRecipient
)
=> bridgeMintCalldata = _burn({
_chainId: ERA_CHAIN_ID,
_nextMsgValue: 0,
_assetId: _assetId,
=> _originalCaller: _originalCaller,
_transferData: abi.encode(_amount, _l2Receiver),
_passValue: false
});

In AssetRouterBase.sol, the _burn function calls IAssetHandler(l1AssetHandler).bridgeBurn:

# AssetRouterBase.sol
function _burn(
uint256 _chainId,
uint256 _nextMsgValue,
bytes32 _assetId,
address _originalCaller,
bytes memory _transferData,
bool _passValue
) internal returns (bytes memory bridgeMintCalldata) {
address l1AssetHandler = assetHandlerAddress[_assetId];
if (l1AssetHandler == address(0)) {
revert AssetHandlerDoesNotExist(_assetId);
}
uint256 msgValue = _passValue ? msg.value : 0;
=> bridgeMintCalldata = IAssetHandler(l1AssetHandler).bridgeBurn{value: msgValue}({
_chainId: _chainId,
_msgValue: _nextMsgValue,
_assetId: _assetId,
=> _originalCaller: _originalCaller,
_data: _transferData
});
}

The BridgedToken is initialized with the assetHandlerAddress[_assetId] as _nativeTokenVault:

function _finalizeDeposit(
uint256 _chainId,
bytes32 _assetId,
bytes calldata _transferData,
address _nativeTokenVault
) internal {
address assetHandler = assetHandlerAddress[_assetId];
if (assetHandler != address(0)) {
IAssetHandler(assetHandler).bridgeMint(_chainId, _assetId, _transferData);
} else {
=> assetHandlerAddress[_assetId] = _nativeTokenVault;
IAssetHandler(_nativeTokenVault).bridgeMint(_chainId, _assetId, _transferData);
}
}

Since the BridgedToken's assetHandlerAddress is _nativeTokenVault, the _burn function will trigger NativeTokenVault::bridgeBurn which calls the _bridgeBurnBridgedToken function:

# NativeTokenVault.sol
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 {
//////////////////////////////////////
}
function _bridgeBurnBridgedToken(
uint256 _chainId,
bytes32 _assetId,
address _originalCaller,
bytes calldata _data
) internal returns (bytes memory _bridgeMintData) {
(uint256 _amount, address _receiver) = abi.decode(_data, (uint256, address));
if (_amount == 0) {
// "Amount cannot be zero");
revert AmountMustBeGreaterThanZero();
}
address bridgedToken = tokenAddress[_assetId];
=> IBridgedStandardToken(bridgedToken).bridgeBurn(_originalCaller, _amount);
_handleChainBalanceIncrease(_chainId, _assetId, _amount, false);

Since the _originalCaller is the user's address, when NativeTokenVault::bridgeBurn triggers _bridgeBurnBridgedToken, it burns the BridgedToken from the user by calling (bridgedToken).bridgeBurn.

As a result, whenever the user bridges the BridgedToken, the contract deducts double the amount of the user's BridgedToken.

  1. L1ERC20Bridge takes the user's BridgedToken by calling transferFrom to transfer the user's BridgedToken into the L1ERC20Bridge contract.

  2. NativeTokenVault burns the BridgedToken from the user.

Impact

  1. If the user still has remaining BridgedToken, they will lose their funds because bridging the BridgedToken causes the contract to take double the amount.

  2. If the user has no remaining BridgedToken, the transaction will fail.

Tools Used

Manual

Recommendations

If user bridge BridgedToken from L1ERC20Bridge contract. Then when executing _bridgeBurnBridgedToken, ensure that the _originalCaller is the address of L1ERC20Bridge, since the user has already transferred their funds to the L1ERC20Bridge.

function depositLegacyErc20Bridge(
address _originalCaller,
address _l2Receiver,
address _l1Token,
uint256 _amount,
uint256 _l2TxGasLimit,
uint256 _l2TxGasPerPubdataByte,
address _refundRecipient
) external payable override onlyLegacyBridge nonReentrant whenNotPaused returns (bytes32 txHash) {
if (_l1Token == L1_WETH_TOKEN) {
revert TokenNotSupported(L1_WETH_TOKEN);
}
bytes32 _assetId;
bytes memory bridgeMintCalldata;
{
// Inner call to encode data to decrease local var numbers
_assetId = _ensureTokenRegisteredWithNTV(_l1Token);
IERC20(_l1Token).forceApprove(address(nativeTokenVault), _amount);
+ if (originChainId[_assetId] != block.chainid) {
+ _originalCaller = address(legacyBridge);
+ }
bridgeMintCalldata = _burn({
_chainId: ERA_CHAIN_ID,
_nextMsgValue: 0,
_assetId: _assetId,
_originalCaller: _originalCaller,
_transferData: abi.encode(_amount, _l2Receiver),
_passValue: false
});
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 6 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.