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

Incorrect Check when removing an NFT collection from whitelisting in `L2Bridge`

alqaqa

Vulnerability Details

When we are adding new elements into whitelists in L2Bridge we are using a Linked list for that mission. To know the ending of the linked list we do two checks. The next element should be either zero or not activated to represent that this is the ending of the linked list.

bridge.cairo#L502-L513

// find last element
loop {
let (_, next) = self.white_listed_list.read(prev);
1: if next.is_zero() {
break;
}
let (active, _) = self.white_listed_list.read(next);
2: if !active {
break;
}
prev = next;
};

As we can see we check that the next is not zero, or it is not active to represent the ending of the linked list.

The problem is that this check is not implemented corectly in case of removing where instead of checking the activation of the next element we are checking the activation of the current element, which is not a correct check to determine the ending of the linked list.

bridge.cairo#L523-L538

// 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 { ... }
};

As we can see we are checking the activation of the prev node, not the next node, which is not the correct check that is implemented when adding new elements.

This is an incorrect logic in the code, where reaching to the end of the array should have the same logic when adding or removing.

Recommendations

Check for the activation of the next node instead of the prev when removing.

diff --git a/apps/blockchain/starknet/src/bridge.cairo b/apps/blockchain/starknet/src/bridge.cairo
index 23cbf8a..d025cb6 100644
--- a/apps/blockchain/starknet/src/bridge.cairo
+++ b/apps/blockchain/starknet/src/bridge.cairo
@@ -527,7 +527,8 @@ mod bridge {
// end of list
break;
}
- if !active {
+ let (activeNext, _) = self.white_listed_list.read(next);
+ if !activeNext {
break;
}
if next == collection {
Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

alqaqa Submitter
9 months ago
n0kto Lead Judge
8 months ago
n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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