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

Broken remove whitelist logic in Cairo

aksoy

Summary

In cairo the code responsible for removing whitelisted collection fails to update the prev value during loop. This leads to an infinite loop, causing the cairo program reach maximum step value and fail.

Vulnerability Details

In the _white_list_collection function, the program enters a loop to remove an element from a linked list. However, the prev value is not updated as the loop progresses. Specifically, after the current prev value is read, it should be updated to the next value. Without this update, the loop fails to advance correctly, resulting in an infinite loop.

// 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;
}
// @audit no update for prev
};
self.white_listed_list.write(collection, (false, no_value));

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

Test:

​
#[test]
fn whitelist_fail_test() {
let erc721b_contract_class = declare("erc721_bridgeable");
​
let BRIDGE_ADMIN = starknet::contract_address_const::<'starklane'>();
let BRIDGE_L1 = EthAddress { address: 'starklane_l1' };
​
let bridge_address = deploy_starklane(
BRIDGE_ADMIN, BRIDGE_L1, erc721b_contract_class.class_hash
);
let bridge = IStarklaneDispatcher { contract_address: bridge_address };
​
start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN);
bridge.enable_white_list(true);
stop_prank(CheatTarget::One(bridge_address));
​
let collection1 = starknet::contract_address_const::<'collection1'>();
let collection2 = starknet::contract_address_const::<'collection2'>();
let collection3 = starknet::contract_address_const::<'collection3'>();
​
start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN);
bridge.white_list_collection(collection1, true);
bridge.white_list_collection(collection2, true);
bridge.white_list_collection(collection3, true);
stop_prank(CheatTarget::One(bridge_address));
​
start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN);
// infinite loop causes fail
bridge.white_list_collection(collection3, false);
stop_prank(CheatTarget::One(bridge_address));
let white_listed = bridge.get_white_listed_collections();
​
}

Test Result:

[FAIL] starklane::tests::bridge_t::tests::whitelist_fail_test
Failure data:
Got an exception while executing a hint: Hint Error: Error in the called contract (0x03b24bdfb3983f3361a7f81e871041cc45f3e1c21bfe3f1cbcaf7bec224627d5):
Error at pc=0:17591:
Could not reach the end of the program. RunResources has no remaining steps.
Cairo traceback (most recent call last):

Impact

Admn cant remove some of the whitelisted collections

Tools Used

Manual

Recommendations

Update prev value inside the loop

// 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;
}
+ prev = next;
};
Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-L2-unwhitelist-from-third collection-impossible

Likelyhood: High, owner can only unwhitelist the 2 first collections. Impact: Medium/High, owner has to empty the list to remove any collection, and replace all the new ones.

Support

FAQs

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