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

`_white_list_collection` and `_whiteListCollection` does not check that whitelisting is enabled could lead to DoS

0xaman

Summary

The Layer 2 bridge contract will whitelist a contract if it is not already whitelisted when withdraw_auto_from_l1 is called. However, it does not check whether whitelisting is enabled.

This issue affects the StarkNet contract, but it also impacts the Ethereum contract.

Vulnerability Details

The _white_list_collection function can be triggered by an admin or when the bridge contract on Layer 2 receives calls from l1_handler. However, it lacks a check to verify whether whitelisting is enabled.

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 above code, we only check if the collection is whitelisted. If it is not, we whitelist it. However, the intended behavior is that if whitelisting is not enabled, any token should be allowed to be bridged.

fn _is_white_listed(self: @ContractState, collection: ContractAddress) -> bool {
let enabled = self.white_list_enabled.read();
if (enabled) {
let (ret, _) = self.white_listed_list.read(collection);
return ret;
}
true
}

From the above function, it can be seen that if whitelisting is not enabled, we return true.

Impact

  1. The collection is added to the whitelist when whitelisting is not enabled, which is not the intended behavior according to the given code.

  2. Whitelisting can be disabled, which allows any user to bridge their tokens. If many token collections are added while whitelisting is not enabled, it could create a DoS for new collections due to the loop used in _white_list_collection. This issue affects both Ethereum and StarkNet contracts.

Tools Used

Manual Review

Recommendations

In the _white_list_collection function, check if whitelisting is not enabled. If so, do not add the collection to the whitelist and simply return.

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.