Era

Summary

When using Bridgehub.sol's requestL2TransactionDirect or requestL2TransactionTwoBridges, it often leads to the function _depositFunds in the NTV.

As shown in the code snippet from the codebase, balanceAfter - balanceBefore is returned and then cross-checked in NativeTokenVault.sol's _bridgeBurnNativeToken :

Hence, the change in balance is cross-checked with what the amount the user initially requested when calling bridgehub.

However, if you follow through the sequence of function from bridgehub to _depositFunds, there's actually another way the user can deposit funds, without triggering _depositFunds.

For context the sequence is:

• bridgehub.requestL2TransactionDirect() -> L1AssetRouter.bridgehubDepositBaseToken() -> NativeTokenVault.bridgeBurn()

Then after that bridgeBurn calls _bridgeBurnNativeToken. The _bridgeBurnNativeToken in L1NativeTokenVault.sol overwrites the original virtual one in NativeTokenVault.sol. The code is as follows:

Since, we can see that in the original virtual _bridgeBurnNativeToken code snippet pasted further up above that _depositFunds is not called if depositChecked is true.

Hence bool depositChecked = IL1AssetRouter(address(ASSET_ROUTER)).transferFundsToNTV() is the line to focus on.

Taking a look at L1AssetRouter's transferFundsToNTV

We can see that the balance after is not checked like how _depositFunds checks it, and it just simply returns true without checking that exactly the amount has been transferred.

Impact no. 1

There are a few common tokens that accepts type(uint256).max passed into the transfer parameters to transfer all of the user's balance. Other than that, it behaves normally just like a standard ERC20 token, meaning there are no fee on transfer when transferring.

So if a user sends bridgehub.requestL2TransactionDirect or bridgehub.requestL2TransactionTwoBridges(if the token isnt the base token of a chain) and passes in type(uint256).max as the requested mint value on L2, the transaction will go through without reverting. (As transferFundsToNTV will just end up transferring the user's balance to the bridge without reverting)

This is problematic as

1. _handleChainBalanceIncrease will cause the chainBalance[_chainId][_assetId] to become type(uint256).max and anymore further deposits of that token will revert permanently

2. Request sent to the L2 would be to mint type(uint256).max tokens for that user which breaks the 1:1 backed invariant of the bridge. Also the user can just use their new increased balance to carry out trades or transactions in the L2 since the token is a legit token on L1

Impact no. 2

Since the protocol's bridge norm invariant is to back tokens bridged to L2 in a 1:1 ratio, it prevents fee on transfer tokens from being bridged.

Being an ecosystem where any user can create chains, the code also actively seeks to prevent the use of fee on transfer tokens from being added and bridged. This can be seen in _depositFunds where the code reverts with revert TokensWithFeesNotSupported() when amount != expectedDepositAmount.

So if users approve their funds to L1AssetRouter instead of the NTV, transferFundsToNTV will run the deposit instead of _depositFunds, bypassing the revert for fee on transfer tokens, allowing it to happen.

The result of that would be the user having more tokens on L2 than they should have (since a part of it was already lost during the transfer to the bridge for locking)

Recommendation

Require that balanceAfter - balanceBefore == amount requested to be minted on L2 in transferFundsToNTV