ArkProject: NFT Bridge

Summary

The bridge _white_list_collection funcion fails when trying to set a collection's whitelist status to false (blacklist) if the collection is already in the enabled (whitelisted) collections list and is not the first or second on the list.

Vulnerability Details

The Starknet bridge _white_list_collection has the following bug: in case a collection's whitelisting status is being changed from enabled=true to enabled=false, and the collection is not at the head of the whitelisted collection list, the following code is executed:

The loop is intended to find the changed collection in the list, and remove it from the list (by linking the element before it to the element after it). However, the loop never progresses the current element. Note that the loop starts with prev (the current head of the list) and then retrieves the next elememt into next. At some point in the loop, prev should be set to next (prev = next;) for the loop to progress, but this never happen. Therefore the code will only work if the exit conditions (next.is_zero() or !active) are met on the first run of the loop (when the collection is the second on the list). If the collection is positioned third or more, the function enters an endless loop and eventually fails.

POC

Add the following test function to bridge_t.cairo and run it:

The function fails because of the endless loop

Impact

Inability to take urgent preventive action when rogue/disfuctional collections are detected on the Starknet bridge (Examples: whitelisted collections that upgrade to malicious code, or to code that breaks bridge functionality causing sent tokens to be permanently locked). Can result in permanent loss of tokens or any other loss the rogue collection may cause.

Tools Used

Manual Review, snForge

Recommended Mitigation

Fix the code mentioned above by adding prev = next; at the appropriate place in the loop.