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

Permanent DoS on L2 due to non-shrinking array usage in an unbounded loop in `bridge::_white_list_collection()`

0xnbvc

Vulnerability Details

The LightChaserV3 bot has found one instance of this vulnerability in the Bridge::getWhiteListedCollections() view function on L1 (see [Medium-1] Permanent DoS due to non-shrinking array usage in an unbounded loop).

However, this bug is also present on the L2 side in the bridge::_white_list_collection() function:

bridge.cairo#L491-L542

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));
}
}
}

This function bridge::_white_list_collection() is called by bridge::ensure_erc721_deployment() (which is called by bridge::withdraw_auto_from_l1()) and by bridge::white_list_collection().

If the white_listed_list state variable becomes too big, the loop will ran out of gas and cause a permanent DoS to bridge::withdraw_auto_from_l1() (and bridge::white_list_collection()), locking the user's tokens. Admins will have to cancel the transaction on L1 and wait the mandatory 5 days delay to recover users' tokens on L1.

Impact

Permanent DoS and severe disruption to the protocol's functionality.

Recommendations

Either limit the size of the white_listed_list state variable, or add a function to remove values from the white_listed_list state variable in case it becomes too big.

Updates

Lead Judging Commences

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

finding-collections-always-withelisted-on-both-chain-withdraw-impossible-collections-array-will-be-OOG

Likelyhood: High, once the whitelist option is disabled, collections will grow. Impact: High, withdraw won’t be possible because of Out-Of-Gas.

Support

FAQs

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