ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect Implementation of `_white_list_collection` Causes Infinite Loop

bladesec

Summary

A critical step is missing in the _white_list_collection function, which leads to an infinite loop. Specifically, the instruction to update the prev variable with the value of next is absent. This omission prevents the loop from progressing as intended, causing the program to become stuck in an endless cycle.

Vulnerability Details

The _white_list_collection function is designed to enable or disable an NFT collection within the system. This function can be triggered through two primary scenarios:

  1. Creation of a New NFT Collection: When a new NFT collection is created and needs to be added to the whitelist.

  2. Admin Control: When an admin wants to enable or disable an existing NFT collection.

While the function correctly handles the enabling of a collection, the logic for disabling a collection is flawed.

Here is the relevant code snippet from the _white_list_collection function:

fn _white_list_collection(ref self: ContractState, collection: ContractAddress, enabled: bool) {
let no_value = starknet::contract_address_const::<0>();
let (current, _) = self.white_listed_list.read(collection);
if current != enabled {
let mut prev = self.white_listed_head.read();
if enabled {
// ... enabling logic
} else {
// change head
if prev == collection {
let (_, next) = self.white_listed_list.read(prev);
self.white_listed_list.write(collection, (false, no_value));
self.white_listed_head.write(next);
return;
}
// removed element from linked list
loop {
let (active, next) = self.white_listed_list.read(prev);
if next.is_zero() {
// end of list
break;
}
if !active {
break;
}
if next == collection {
let (_, target) = self.white_listed_list.read(collection);
self.white_listed_list.write(prev, (active, target));
break;
} // <-- Missing: prev = next
};
self.white_listed_list.write(collection, (false, no_value));
}
}
}

In the code, the function first checks if the current status of the collection is different from the desired status (enabled). If the statuses differ, the function attempts to update the whitelist configuration.

  • Enabling Logic: When the collection is to be enabled, the function works correctly.

  • Disabling Logic: The issue arises when the function tries to disable a collection. Inside the loop that traverses the linked list, the prev variable is supposed to be updated with the value of next so that the loop can move forward to the next element in the list. However, this crucial step is missing, causing the loop to repeatedly check the same element, leading to an infinite loop.

Impact

Due to this infinite loop, an admin is unable to disable an NFT collection. This bug effectively locks the admin out of performing this action, which could have severe implications for the manageability and functionality of the NFT collections.

Tools Used

Manual Review

Recommendations

To resolve this issue, the missing line of code prev = next should be added at the end of the loop. This will ensure that the prev variable is correctly updated, allowing the loop to proceed to the next element and eventually exit as expected.

Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-L2-unwhitelist-from-third collection-impossible

Likelyhood: High, owner can only unwhitelist the 2 first collections. Impact: Medium/High, owner has to empty the list to remove any collection, and replace all the new ones.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.