Summary
The _bridgeBurnNativeToken function calls transferFundsToNTV to transfer funds from the _originalCaller, and then calls _depositFunds, which results in the same funds being deducted twice from the user's account. This causes the user to be charged more than intended, leading to potential loss of funds and malfunctioning of the bridge operation.
Vulnerability Details
The _originalCaller is charged twice for the same transaction. First, funds are transferred to the Native Token Vault (NTV) via the transferFundsToNTV function. Then, the same funds are again transferred from the _originalCaller using the _depositFunds function.
This double deduction occurs without proper checks, leading to an unintended financial loss for the user and unexpected behavior in the transaction process.
First, when transferFundsToNTV transfers funds to the Native Token Vault (NTV).
Then, when _depositFunds is called, funds are transferred from the _originalCaller again, resulting in double the intended deduction.
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;
if (_depositAmount == 0) {
_depositAmount = amount;
}
_handleChainBalanceIncrease(_chainId, _assetId, amount, true);
if (_depositAmount != amount) {
revert ValueMismatch(_depositAmount, amount);
}
} else {
if (msg.value != 0) {
revert NonEmptyMsgValue();
}
amount = _depositAmount;
_handleChainBalanceIncrease(_chainId, _assetId, amount, true);
if (!_depositChecked) {
uint256 expectedDepositAmount = _depositFunds(_originalCaller, IERC20(nativeToken), _depositAmount);
if (amount != expectedDepositAmount) {
revert TokensWithFeesNotSupported();
}
}
}
if (amount == 0) {
revert EmptyDeposit();
}
bytes memory erc20Metadata = getERC20Getters(nativeToken, originChainId[_assetId]);
_bridgeMintData = DataEncoding.encodeBridgeMintData({
_originalCaller: _originalCaller,
_remoteReceiver: _receiver,
_originToken: nativeToken,
_amount: amount,
_erc20Metadata: erc20Metadata
});
emit BridgeBurn({
chainId: _chainId,
assetId: _assetId,
sender: _originalCaller,
receiver: _receiver,
amount: amount
});
}
function _depositFunds(address _from, IERC20 _token, uint256 _amount) internal virtual returns (uint256) {
uint256 balanceBefore = _token.balanceOf(address(this));
_token.safeTransferFrom(_from, address(this), _amount);
uint256 balanceAfter = _token.balanceOf(address(this));
return balanceAfter - balanceBefore;
}
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);
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;
}
Impact
The transferFundsToNTV function and _depositFunds function both transfer funds from the _originalCaller address without ensuring that only one of these transfers is needed. This results in duplicate transfers of the same amount, causing financial discrepancies.
Tools Used
Recommendations
Add a condition to check if the funds were already transferred via transferFundsToNTV. If the funds have already been transferred, _depositFunds should not be called again for the same amount.
