Summary
In the _white_list_collection(...)function, it is possible to whitelist zero address when disabling a collection on L2
Vulnerability Details
As show below on L535 and L536, the function does not check if the targetis a zero address. This is possible if the collection being disabled is the last on the list. In this situation, target == address(0)
File: bridge.cairo
492: fn _white_list_collection(ref self: ContractState, collection: ContractAddress, enabled: bool) {
493: let no_value = starknet::contract_address_const::<0>();
494: let (current, _) = self.white_listed_list.read(collection);
495: if current != enabled {
496: let mut prev = self.white_listed_head.read();
497: if enabled {
SNIP ........
516: } else {
517:
....... .......
524:
525: loop {
..... .......
534: if next == collection {
535: @> let (_, target) = self.white_listed_list.read(collection);
536: @> self.white_listed_list.write(prev, (active, target));
537: break;
538: }
539: };
540: self.white_listed_list.write(collection, (false, no_value));
541: }
542: }
Impact
Null or address(0) can be whitelisted.
Tools Used
Manual review.
Recommendations
Modify the _white_list_collection(...)function as shown below
File: bridge.cairo
492: fn _white_list_collection(ref self: ContractState, collection: ContractAddress, enabled: bool) {
493: let no_value = starknet::contract_address_const::<0>(); // address zro
494: let (current, _) = self.white_listed_list.read(collection); // (curent status of the collection, next after the collection)
495: if current != enabled { // if tthe collection does not already have the status it is being set to
496: let mut prev = self.white_listed_head.read(); // the current address at the whitelist head
497: if enabled { // setting to true
SNIP ........
516: } else { // setting to false (blacklisting)
517: // change head
....... .......
524: // removed element from linked list
525: loop {
..... .......
534: if next == collection {
535: let (_, target) = self.white_listed_list.read(collection);
-536: self.white_listed_list.write(prev, (active, target));
+ if !target.is_zero() {
+ self.white_listed_list.write(prev, (active, target));
+ }
537: break;
538: }
539: };
540: self.white_listed_list.write(collection, (false, no_value));
541: }
542: }
