ArkProject: NFT Bridge

Low-1: DOS: As the _collections array size increases, bridging non-escrow tokens from l2-l1 will dos due to unlimited memory reads in loop

When an new non-escrowed Nft is bridged from starknet to the L1 for the first time, a representative nft is minted against it and at the same time added to the whitelist. But the whitelisting operations first loops through the _collections array to check if the nft is already whitelisted.

Since traversing through arrays is one of the most gas expensive operations, The bridging process will DOS in case where this looping operations costs more gas than the forwarded gas limit.The best option would be to use a mapping data structure to represent if the nft is whitelisted or not instead of looping through the array.

The worse part is when the whitelist mode is enabled, an attacker can bridge ton of self-created nfts from L2-L1 in order to exhaust the _collections array in bridge.sol, so that newly bridged legit tokens result in the DOS.

Low-2: Attacker can easily DOS the withdrawals of all owners of any ERC1155 token id.

While withdrawing ERC1155 token id on L1 (bridge.sol), it is checked whether the nft id is currently in escrow.

But once the user withdraws any token balance of particular ERC1155 token id, the whole ERC1155 id collection is considered to be unavailable in escrow.
That means all other owners of that ERC1155 token id collection will not be able to withdraw their assets

Attack path:

• alice, bob, and john each own 10,000 tokens of ERC1155 token with id 5.

• All of them bridge them to L2 and the escrow mapping is updated.

• Attacker buys the same ERC1155 token with id no 5, and withdraws

• since the withdrawal process incorrectly updates the whole id to be unescrowable := _escrow[collection][id] = address(0x0);

• Alice, bob and john won't be able to withdraw their token balances.

Since the tokens can be recovered by depositing another id 5 ERC1155 token, but the attacker can use bots to constantly make the withdrawals DOS, all he has to do is just contantly withdraw 1 token.

since the attack is costly , i think the severity to be Low but don't forget all it takes is couple of unsatisfied and scared customers to view the landscapes of bankruptcy

Low-3: Follow the same L1 pattern for setting l1-l2 collection mappings in bridge.cario, ortherwise users can loose acess to their original asset

As we know, when new tokens are bridged either from l1-l2 or l2-l1, their representative nft is minted on either side and their relation is recoreded in the collection mapping. the problem arises when we bridge back representative nft for the original one, as the both sides first check whether the token has some collection mapping, since the collection mapping will only be updated on the one side while bridging for the first time.

That is why, the external setter has been implemented on both sides to set mapping collections.

The problem is , on bridge.cairo it doesn't verify whether the collection mappings has already been set, which could result in the loss of user retrieving his funds, if the admin changed collections.

Follow this implementation