NFTBridge
60,000 USDC
View results
Submission Details
Severity: high
Valid

Bridging too many nft from Mainnet to Starknet can cause out of gas error in mainnet and no more nft can be bridged.

Title

Bridging too many nft from Mainnet to Starknet can cause out of gas error in mainnet and no more nft can be bridged.

Line of code

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

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

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

Vulnerability Details

User can bridge nft from Mainnet to Starknet. On L2, the function _white_list_collection() will be called.

If we see the withdraw_auto_from_l1() function, it calls the ensure_erc721_deployment()

#[l1_handler]
fn withdraw_auto_from_l1(
ref self: ContractState,
from_address: felt252,
req: Request
) {
...
...
@> let collection_l2 = ensure_erc721_deployment(ref self, @req);
...
...
}

For erc721 deployment it calls the _white_list_collection() if the already_white_listed != true

fn ensure_erc721_deployment(ref self: ContractState, req: @Request) -> ContractAddress {
...
...
// update whitelist if needed
let (already_white_listed, _) = self.white_listed_list.read(l2_addr_from_deploy);
if already_white_listed != true {
@> _white_list_collection(ref self, l2_addr_from_deploy, true);
self.emit(CollectionWhiteListUpdated {
collection: l2_addr_from_deploy,
enabled: true,
});
}
...
...
}

Now the issue arises in the function _white_list_collection() :

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

From the above function, Basically the code needs to iterate over the linked list and add one element to listed list and it can leads to out of gas error in Starknet. The block gas limit is 5M in starknet.

basically the malicious attacker can keep minting and bridging a lot a lot of worthless nft to grow the linked list size to block user bridging and make user's NFT withdraw request not able to finalize which cause loss of fund.

Impact

Due to iteration over linked list add one element to listed list and it can leads to out of gas error of whitelisted collection addresses, the user's NFT will be stuck in L1(Mainnet) due to DOS of the function.

Tools Used

Manual Review

Recommendations

We recommend to change the design logic of adding the _white_list_collection addresses.

Updates

Lead Judging Commences

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