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

Bridging too many nft from `Starknet` to `Mainnet` can cause out of gas error in Mainnet and no more nft can be bridged.

asefewwexa

Title

Bridging too many nft from Starknet to Mainnet can cause out of gas error in Mainnet and no more nft can be bridged.

Line of code

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L115

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L340

Vulnerability Details

It is possible for user to bridge nft from Starknet to Ethereum directly. On L1, the function _whiteListCollection() will be called.

function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
if (collectionL1 == address(0x0)) {
if (ctype == CollectionType.ERC721) {
collectionL1 = _deployERC721Bridgeable(
req.name,
req.symbol,
req.collectionL2,
req.hash
);
// update whitelist if needed
@> _whiteListCollection(collectionL1, true);
} else {
revert NotSupportedYetError();
}
}
}

Then if collectionL1 is address(0) it means that the first time the starknet nft is bridged from L2 to L1 and for every bridge transaction the first time the starknet nft is bridged from L2 to L1, then the code aims to whitelist the collection after deploying the NFT if we see the function StarkLane::_whiteListCollection()

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;
}
​

Now, It would requires looping over all the collection array and push one element to the collection array. Then eventually this for loop will become unbounded and running out of gas and block user from finalize withdraw in L1. But user's NFT is already in Starknet, then it result in loss of fund.

Impact

Due to unbounded loop of whitelisted collection addresses, the user's NFT will be stuck in Starknet due to DOS of the function.

basically the malicious attacker can keep minting and bridging a lot a lot of worthless nft to grow the collection array size to block user bridging and make user's NFT withdraw request not able to finalize which cause loss of fund.

Tools Used

Manual Review

Recommendations

We recommend to change the design logic of adding the _whiteListCollection addresses.

Updates

Lead Judging Commences

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