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

Infinite loop in `bridge.cairo::_white_list_collection`, element in the middle cannot be removed from the collection whitelist

simon0417

Relevant Github Link

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/starknet/src/bridge.cairo#L537

Summary

The loop inside the _white_list_collection function will never break as it does not correctly iterate through the list. As a result, the contract owner cannot remove elements from the middle of the whitelist.

Vulnerability Details

​
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 {
self.white_listed_list.write(collection, (enabled, no_value));
if prev.is_zero() {
self.white_listed_head.write(collection);
return;
}
loop {
let (_, next) = self.white_listed_list.read(prev);
if next.is_zero() { //if no next element
break;
}
let (active, _) = self.white_listed_list.read(next);
if !active {
break;
}
prev = next;
};
self.white_listed_list.write(prev, (true, collection));
} else {
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;
}
loop {
let (active, next) = self.white_listed_list.read(prev);
if next.is_zero() {
break;
}
if !active {
break;
}
if next == collection {
let (_, target) = self.white_listed_list.read(collection);
self.white_listed_list.write(prev, (active, target));
break;
}
@>
};
self.white_listed_list.write(collection, (false, no_value));
}
}
}
​

The loop is expected to break when the element is the last in the list or matches the target element, but the variable prev is never updated to the next element. This causes an infinite loop.

Impact

Likelihood: High

This issue will always occur when the admin tries to remove an element from the whitelist, unless it is the first element in the list.

Impact: Low

The admin will not be able to directly remove a whitelisted collection from the list when necessary.

Tools Used

Manual Review

Recommendations

loop {
let (active, next) = self.white_listed_list.read(prev);
if next.is_zero() {
break;
}
if !active {
break;
}
if next == collection {
let (_, target) = self.white_listed_list.read(collection);
self.white_listed_list.write(prev, (active, target));
break;
}
+ prev = next;
​
};
​
Updates

Lead Judging Commences

n0kto Lead Judge 9 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.