ArkProject: NFT Bridge

Summary

The depositTokens() function in L1 bridge does not explicitly check if a valid mapping exists in the _l1ToL2Addresses mapping for the provided collectionL1 address.
This means that if the contract owner has not set up the mapping properly using the setL1L2CollectionMapping() function or the tokens are deposited before setting up of the mapping, the depositTokens() function will still attempt to use the default (likely zero) address for req.collectionL2.

Vulnerability Details

The depositTokens function is called with an L1 collection address (collectionL1):

The function attempts to retrieve the corresponding L2 address (collectionL2) from the l1ToL2Addresses mapping without checking if the mapping exists. It directly assigns the retreived address to the request object without validating req.collectionL2.:

If the user calls this function and the mapping doesn't exist (i.e., if the owner hasn't set it up using setL1L2CollectionMapping), this will default to the zero address (0x0) for req.collectionL2.

The function proceeds to call sendMessageToL2 with this potentially invalid L2 address:

If the req.collectionL2 is set to an invalid address (e.g., the zero address), the sendMessageToL2() function will still accept this value and proceed to process the message and emit an event:

When this message is processed on the L2 (Starknet) side, it will likely fail due to the invalid collection address.

This could result in the user's tokens being locked in the escrow on L1, as the bridge transfer to Starknet will not complete successfully.

Impact

Users' tokens gets locked in the L1 escrow, as the L2 side won't be able to complete the transfer process with an invalid collection address.

Tools Used

Manual Review

Recommendations

Add a validation check in the depositTokens() function to ensure that a valid mapping exists for the provided collectionL1 address before proceeding with the token deposit and transfer.