ArkProject: NFT Bridge

Summary

A user can DoS the deployment of new ERC721Bridgeable contracts on Ethereum if whitelisting is disabled on Starknet

Vulnerability Details

The flow to bridge an NFT from Starknet to Ethereum is the following:

1. Call the deposit_tokens in the Starknet bridge

2. The bridge internally send a message to the starknet core to deliver the message to Ethereum

3. Once enough time passed, the message gets delivered in Ethereum and can be consumed

Once the message reaches Ethereum, this code snippet will be executed:

Essentially, when an NFT of a collection that has not ever been bridged, a new instance of ERC721 has to be deployed and whitelisted into the protocol. Let's see how the whitelisting function works in more depth.

The function to whitelist a collection basically loops through an array of whitelisted addresses to see if it is already present in the array. If it is not present, it pushes it and finally sets a mapping to either true or false depending on the enable parameter. The problem here is that this function always increase, never gets smaller because when the owner will try to dewhitelist a collection will call this function:

But it will only set the mapping to false without poping the collection from the _collections array.
The problem here is that once the array is significantly large, looping through this array to add a new collection to whitelist will run out of gas and the transaction will revert.

This state can be maliciously driven by a user if the bridge in Starknet has the whitelisting feature disabled, hence any NFT collection can be bridged. That is because he can spam the creation of NFT collection for free and bridge a single NFT of each collection to Ethereum. When all the messages will arrive, they will fill the L1 collections array when the user withdraws each NFT because it will register all the collections to the list. Once the array has increased enough he disabled the possibility of bridging any new NFT collection and blocked also the possibility for the owner to manually whitelist new collections.

Regarding the issue in the opposite way, sending spam Ethereum NFTs to Starknet, it will work the same, however, the owner of the bridge will be able to effectively dewhitelist a collection and remove it from the data structure, in this case a linked list. So taking into account that the owner can make the linked list shrink I would not consider there a bug.

Impact

If the array reaches a state where looping through it is not possible due to running out of gas, the following things will stop working:

• When a new NFT collection from Starknet will be bridged to Ethereum it will not be possible to be withdrawn because the _whiteListCollection will be called and will revert. Thus, the Starknet NFT will be blocked forever.

• When the bridge owner want to manually whitelist a new collection it will not be possible due to the same issue.

The impact of this issue is high because the state of DoS can be reached almost for free by a normal user or by the normal utilization of the bridge will eventually reach this state.

Tools Used

Manual review

Recommendations

The only functionality of the collections array is to have a view function with the currently whitelisted collections. From my point of view, it makes no sense to have this function because with just a mapping storing a boolean would be enough. However, if you want to preserve this feature, the best solution would be to pop the element from the array when the owner dewhitelists a collection.