ArkProject: NFT Bridge

Github
https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L192

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L352

Summary

If _collections grows unbounded, it could lead to high gas costs for operations that iterate through this array. This could impact the efficiency and cost of future whitelisting operations.

Vulnerability Details

The _whiteListCollection function is responsible for managing the _collections array. This function is called during token withdrawals if the collection does not exist on L1, and also by the admin to enable or disable collections. Users bridging messages from L2 to L1 and admins adding entries can cause _collections to grow indefinitely. As the array expands, operations that iterate through it will become more expensive, resulting in higher gas costs for future whitelisting operations.

This issue becomes more risky when the global whitelist is disabled, because when global whitelist is disabled the protocol allow any collection tokens to be deposited.

Impact

The growing size of _collections can lead to increased gas fees for whitelisting operations, affecting user costs. This inefficiency could also result in transaction failures or scalability issues.

Recommendation

Regularly review and clean up the _collections array to ensure it does not grow excessively. Currently when disabling the collection it doesn't remove it from collections array.

Use more efficient data structures or indexing methods to handle large collections.

Also, I think it would be better to add a check in the withdrawTokensto not call _whiteListCollectionwhen global whitelist is disabled. Because when global whitelist is disabled the protocol allow any collection to deposit and withdraw, so this check should only be called when the global whitelist is enabled.