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

in function `_white_list_collection` disabled collections might be allowed to be treated as whitelisted

minato7namikazi

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;
}
// find last element
loop {
let (_, next) = self.white_listed_list.read(prev);
if next.is_zero() {
break;
}
let (active, _) = self.white_listed_list.read(next);
if !active {
break;
}
prev = next;
};
self.white_listed_list.write(prev, (true, collection));
} 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;
}
};
self.white_listed_list.write(collection, (false, no_value));
}
}

in the _white_list_collection function:

When removing a collection from the whitelist (setting enabled to false), the function only updates the linked list if the collection is at the head or somewhere in the middle. It fails to handle the case where the collection to be removed is at the end of the list.

This could lead to a situation where a disabled collection at the end of the list is still considered part of the whitelist when iterating through it, as the previous element would still point to it.

This bug could cause issues with the whitelist management, potentially allowing disabled collections to be treated as whitelisted in some scenarios.

Impact

Flow/PoC:

  1. Several collections are whitelisted.

  2. The last collection in the list is removed from the whitelist using white_list_collection(last_collection, false).

  3. Due to the bug, this collection remains linked in the list.

  4. When deposit_tokens is called, it checks if the collection is whitelisted using _is_white_listed.

  5. _is_white_listed uses get_white_listed_collections, which iterates through the linked list.

  6. The removed collection is still in this list, so it's considered whitelisted.

  7. This allows tokens from a supposedly non-whitelisted collection to be bridged.

This vulnerability could be exploited to bridge tokens from collections that should be restricted, potentially leading to unauthorized asset transfers between L1 and L2

Recommendations

To fix this, the function should ensure that when removing the last element in the list, the previous element's "next" pointer is set to zero. This would require keeping track of the previous element throughout the entire loop, not just when finding the collection to remove.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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