Vulnerability Details

When Bridging NFTs from L1 -> L2, we are doing some checks. This includes ensuring a supported ERC721 interface. and tokens to be bridged be greater than 0.

But When bridging from L2 -> L1, these two checks do not exist. we neither detect the interface nor check that the tokenIds is greater than zero.

1. When we are getting erc721_metadata without checking interface support, which is named introspection or ERC165 in EVM. And we are doing the function with Option returned value. So if there is an error occur when calling token_uri we just return an empty string.

bridge.cairo#L266-L270

And since most ERC20 tokens implement the name()and symbol() like that in ERC721 tokens

collection_manager.cairo#L67-L74

So if we passed an ERC20 token

2. When we send tokens to the L2Bridge via escrow_deposit_tokens, we are not checking that tokens to be bridged are greater than zero, which is not the case in L1Bridge where there is a check to ensure tokens are greater than zero.

L2Bridge:
bridge.cairo#L402-L407

L1Bridge:
Escrow.sol#L33

So after collecting these two issues together we will be able to pass any address as collection_l2, and the tx will get processed successfully.

A side issue

The first issue is that 0 tokenIds amount is not checked in L2Bridge which is not the case in L1Bridge this is a side LOW severity issue, where tokenIds greater than 0 invariant is not checked in L2Bridge, which will allow users to create NFTs collection on L1 and mapp them into the L2 NFT collections without sending a single token.

Returning back to our issue, we proved that an ERC20 token will be passed till it reaches _depositIntoEscrow(). If tokenId is zero as we illustrated in the side issue, then matching an ERC20 on L2 to an ERC721 on L1 will occur. But the issue is not just 0 tokens, since transfer_from(address,address,u256) is the function signature used when transfering ERC721 tokens, this function is exactly the same for ERC20 tokens in Starknet, so The user can simply but tokenID with any value and that value of tokens will get transfered from him to the L2 Bridge.

Example:

• tokenId = 1, will send 1 wei from the user to the Bridge

• tokenId = 1e18 will send 1e18 token amount from the user to the Bridge

And the function will successed and that user will be able to receive an NFT on L1 represents the amount he deposited on L2. and we can simply withdraw then by briding an NFT from L1 to L2 to receive the ERC20.

This will result in deploying an ERC721 collection on l1 and tie it with that ERC20 address passed as collection_l2 by the called of L2Bridge::deposit_tokens(), which breaks the invariant of Linking NFTs collection on L1 and L2 and will link L1 NFT collection with ERC20 address on L2.

Impact

Linking ERC20 tokens on L2 to L1 addresses on L1

Tools Used

Manual Review

Recommendations

For solving the side issue: Check that the tokens to be bridges is greater than zero, this will ensure that this is an NFT collection as we are calling erc721.transfer_from and if did not support this interface it will revert the tx.

And for the main issue, we should implement the SRC5 check that is like ERC165 in solidity, to insure that the address is an ERC721 not an ERC20 address.

NOTEL these are two separate issues not related to each other the first one is about not checking tokenIds array to be greater than zero, and the other is that not checking the interface of the address passed will allow linking ERC20 tokens on L2 Bridge as an NFTs on L1.