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

An attacker can DoS the L1 bridge post deployment as un-whitelisted collections cannot be removed from the `_collections[]` array

sammy

Summary

Un-whitelisted collections are not removed from the _collections[] array, leading to higher gas fees for withdrawTokens() transactions. This can also lead to a permanent DoS if the array size becomes too big, as there is no way of shrinking the array size.

Vulnerability Details

The _whiteListCollection() is called by the whiteList() and withdrawTokens() functions. It adds a collection to the _collections[] array if it is being whitelisted for the first time. However, it does not remove a collection from the array when the collection is un-whitelisted. Since this function is called every time a new collection is whitelisted through whiteList() and withdrawTokens(), it can DoS these functions or make them increasingly expensive to call.

function _whiteListCollection(address collection, bool enable) internal {
if (enable && !_whiteList[collection]) {
bool toAdd = true;
uint256 i = 0;
while(i < _collections.length) {
if (collection == _collections[i]) {
toAdd = false;
break;
}
i++;
}
if (toAdd) {
_collections.push(collection);
}
}
_whiteList[collection] = enable;
}

At deployment, on the L2 bridge, the white_list_enabled bool is set to false :

fn constructor(
ref self: ContractState,
bridge_admin: ContractAddress,
bridge_l1_address: EthAddress,
erc721_bridgeable_class: ClassHash,
) {
self.ownable.initializer(bridge_admin);
// TODO: add validation of inputs.
self.bridge_l1_address.write(bridge_l1_address);
self.erc721_bridgeable_class.write(erc721_bridgeable_class);
self.white_list_enabled.write(false);
self.enabled.write(false); // disabled by default
}

This means that any collection's NFT can be bridged from L2 to L1. An attacker can take advantage of this and fill up the _collections[] array by bridging multiple collections' NFT from L2 to L1. This will DoS the withdrawTokens() function on the L1 bridge. Hence any honest user won't be able to withdraw their NFTs on L1 bridge and they will be lost forever.

Impact

Protocol DoS + Loss of NFTs

Tools Used

Manual Review

Recommendations

_whiteListCollection() can be altered as follows :

function _whiteListCollection(address collection, bool enable) internal {
if (enable && !_whiteList[collection]) {
bool toAdd = true;
uint256 i = 0;
while(i < _collections.length) {
if (collection == _collections[i]) {
toAdd = false;
break;
}
i++;
}
if (toAdd) {
_collections.push(collection);
}
}
else if (!enable && _whiteList[collection]){
uint256 i = 0;
​
while(i < _collections.length){
if(collection == _collections[i])
{
_collections[i] = _collections[_collections.length - 1];
_collections[_collections.length - 1] = collection;
_collections.pop();
break;
}
i++;
}
}
​
_whiteList[collection] = enable;
}
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year 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.