ArkProject: NFT Bridge

Summary

The _collections array in Bridge.sol is only appended to and never pruned, which can lead to a denial of service (DoS) due to gas limitations when interacting with this array.

Vulnerability Details

In Bridge.sol, elements are ONLY ADDED into the array _collections, and these are not removed in any other function, when they actually should be. This can lead to DOS. The following function will reach a limit of gas in a block when _collections.length ≈ 12_000

An attack on this function would be as follows :

1. User deploys multiple contracts on l2 and bridge an NFT to l1. ( whitelist flag is by default set to false )

2. User withdraws for each NFT contract on L1.

3. The function withdrawTokens will always call whitelistCollection here, even when the whitelist flag is set to false, and will set the mapping to true.

4. The user can increase the size of the _collections array by deploying more contracts on L2 and withdrawing them on L1.

5. when the _collections array reaches a certain size, the function _whiteListCollection will reach the gas limit and the function will be unusable for new NFTs deployed on L2.

6. Even though the owner is able to set the whitelist flag to false, or set an address to non whitelisted, the function _whiteListCollection will still be unusable because the length of the _collections array cant be reduced.

Location:

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

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

Impact

The probability of this issue occurring is medium, but the impact is high. The function _whiteListCollection becomes unusable when the array size exceeds a certain limit, causing the contract to be unable to handle new whitelisting operations causing denial of service

Tools Used

• Manual code review

Recommendations

• it seems the array _collections is only used for reading purposes on getWhiteListedCollections() , rather than using an array, consider just using the already existing mapping and use a counter to keep track of the number of collections. When reading just iterate over the mapping and return the keys with value true.