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

Permanent (L1 -> L2) DoS Because of whitelisting Linked List Logic.

Vulnerability Details

When doing a Bridged transaction (L2 -> L1), the sequencer calls L2Bridge::withdraw_auto_from_l1.

When calling this function in the last of it we call ensure_erc721_deployment and in the last of this function, we call _white_list_collection if the collection is not whitelisted we add it to the whitelisted Linked List.

bridge.cairo#L471-L478

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

If we check the logic of the addition in _white_list_collection we will find that it adds the new collection in the last of that Linked List, and we are using single Linked List DS, not Double. So we are iterating over all the elements to put this NFT collection in the Linked List.

bridge.cairo#L502-L514

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

As we can see we are iterating from the head to the next element to the next one, and so on till we reach the end of the linked list (the next element is zero), then we link our collection to it.

Whitelisting a linked list is only a part of the logic done when calling withdraw_auto_from_l1, besides deploying the NFT collection, mint NFTs to the L2 recipient, and other things, so the tx gas cost will increase and may even reach the maximum.

Another thing we need to take into consideration is that this function is called by the sequencer, so reverting the tx is possible, and the sequencer may not accept the message from L1 from the beginning if the fees provided was small.

Proof of Concept

Normal Scenario

  • WhiteListing is disabled in L1Bridge, so any NFT collection can be bridged.

  • Bridge is so active and a lot of NFTs are bridged from L1 to L2.

  • Once the collection is deployed, it is added to whitelisted collection Linked List.

  • The number of whitelisted collections increases 10, 20, 50, 100, 200, 500, 1000, ...

  • The cost for Bridging new NFT collection became too large as withdraw_auto_from_l1 iterates 1000 time besides deploying a new contract and some checks.

  • The transaction will get reverted when the sequencer calls it because of the huge TX cost, or it will not accept processing the message in the first place from L1.

Attack Scenario

  • WhiteListing is disabled in L1Bridge, so any NFT collection can be bridged.

  • Bridge is so active and a lot of NFTs are bridged from L1 to L2.

  • An Attacker spammed Bridging different NFTs from (L1 -> L2).

  • The number of whitelisted collections increases 10, 20, 50, 100, 200, 500, 1000, ...

  • The cost for Bridging new NFT collection became too large as withdraw_auto_from_l1 iterates 1000 time besides deploying a new contract and some checks.

  • The transaction will get reverted when the sequencer calls it because of the huge TX cost, or it will not accept processing the message in the first place from L1.

Impact

Permanent DoS for Bridging new NFTs from L1 -> L2

Tools Used

Manual Review

Recommendations

You can use something like OpenZeppelin EnumberableSets in solidity, and if it is not found we can use Head/Tail Linked List, where we will store the First and the last element in the linked list, so when we add a new element we will do it at θ(1) not θ(N) Average time complexity.

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.