Summary

The protocol implements a whitelist functionality, which, when enabled, allows the admin to specify which collections can be used in the protocol. The issue is that this whitelist functionality traverses an array in a way that can easily exceed the maximum gas limit for a transaction on Ethereum—30 million. This could result in a DoS attack on the bridge and lock users' tokens.

Vulnerability Details

The problem lies in the _whiteListCollection function, which is used to add/remove collections from the whitelist. In this function, the _collections array is traversed in a while loop to check if the collection passed as a parameter is already in the array. If it's not, it is added. Then, the flag of the corresponding collection is changed. It's important to note that elements are not removed from the collections array; they are only added, meaning it will constantly grow. Adding a new element to it results in traversing all elements in the array. An existing element leads to traversing part of the array until it is found through linear traversal.

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

Exceeding 30 million gas consumed by the _whiteListCollection function alone is reached with about 12,000 entries in the collections array (as seen from the provided PoC below). However, since other functions are also executed, the value will be lower. For comparison, according to OpenSea's website, there are over 30 million registered collections, so 12,000 is an insignificant number compared to the NFT market. It's worth noting that collections are created and added to the whitelist when a token is transferred from an L2 collection that does not exist on L1.

In this context, I want to describe another bug that can be used by a malicious actor to accelerate the execution of the attack. In the L2 implementation of the deposit_tokens function, it's possible to pass an empty token_ids array. This is not validated, and when tokens are transferred, the array is traversed, but if it is empty, no error is returned. Therefore, if there is a certain amount of whitelisted collections on L2, a malicious actor can send messages with empty token_ids to L1, which will lead to the deployment of the corresponding collections and their whitelisting there. This way, the malicious actor can save money on purchasing tokens from each of the collections in order to send them.

The _whiteListCollection function is called from withdrawTokens and from whiteList. If a DoS occurs in this function, new collections cannot be added, and in some cases, existing collections cannot be delisted. Also, tokens sent from L2 for collections that do not exist on L1 will remain locked on L2.

Finally, I want to note that in the report from LightChaser, there is a report related to an issue caused by traversing a large array, but it concerns a view-only function. The problem described here is not shown in the LightChaser report.

POC

Run this test with the following command to see that the gas consumption exceeds 30M:

forge test --mt test_dos_whitelist --gas-report --gas-limit 999999999999999

Impact

Lock of funds and dos. User will loss funds because of large gas fees when they call withdrawTokens even if it doesn't revert.

Tools Used

Manual review

Recommendations

Consider removing of this array, it is not necessary for the main functionality.