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

Misconfiguring whitelist linked list when adding an element

fyamf

Summary

Protocol implemented if !active {break;} during adding an active element to the linked list to handle the situation properly if an inactive element by mistake (or any other bugs) is present in the linked list. But, it is not handling properly, and it leads to two situations:

  • The wrong inactive element would not be corrected and removed.

  • The active element before the to-be-added element becomes unlinked.

Vulnerability Details

Suppose the linked list as follows:

  • collection1: true, next: collection2

  • collection2: false, next: collection3

  • collection3: true, next: 0

The goal is to add collection4 as true. During the iteration, when it reaches to collection2, due to its inactivity, it breaks the loop. Then, after the loop, collection1 is linked to collection 4, moreover collection3 becomes unlinked. The final status would be:

  • collection1: true, next: collection4

  • collection2: false, next: collection3

  • collection3: true, next: 0

  • collection4: true, next: 0

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

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

The correct status when the body of if !active {...} is executed should be:

  • collection1: true, next: collection3

  • collection2: false, next: 0

  • collection3: true, next: collection4

  • collection4: true, next: 0

Please note that, it must never happen to have an inactive element in the linked list, in other words, the body of if !active {...} should never be executed. If it happened, it means another part of the code is broken. If for any reason (other part of the code is buggy, or an update misconfigures the list) the body of if !active {...} is executed, it does not handle the situation properly as explained above, and it leads to another bug.

Impact

  • Breaking the linked list.

Tools Used

Recommendations

It should be modified as follows:

loop {
let (_, next) = self.white_listed_list.read(prev);
if next.is_zero() {
break;
}
let (active, _) = self.white_listed_list.read(next);
if !active {
+ self.white_listed_list.write(next, (false, not_value));
+ let (_, secondNext) = self.white_listed_list.read(next);
+ self.white_listed_list.write(oldPrev, (true, secondNext));
+ prev = secondNext;
break;
}
prev = next;
+ oldPrev = prev;
};
self.white_listed_list.write(prev, (true, collection));

Or simply add an assert:

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;
- }
+ assert(active, "should not have inactive element in the linked list");
prev = next;
};
self.white_listed_list.write(prev, (true, collection));
Updates

Lead Judging Commences

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