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

Native NFT tokens will be locked if bridge back

jokrsec

Summary

Due to incorrect implementation of verification logic for collection addresses while withdrawing, NFTs will get locked forever

Vulnerability Details

L1 <-> L2 mappings are updated with newly deployed ERC721 bridgable contract addresses if it is the first token of a collection. But as these mappinps would only be updated on the withdraw functions, it leaves the mapping from where the tokens are deposited unset. This would result in error while verifying collections address while calling withdraw

Lets say alice PLAY is a new whitelisted collection on L1

Alice bridges PLAY #20 to bob. This is the first token being bridged of this collection.
As _l1ToL2Addresses[PLAY] returns 0 address, there will be corresponding bridgeable collection will be deployed. Lets call it PLAY_ON_L2

Now the mapping on L2 will be updated as

l1_to_l2_addresses[PLAY, PLAY_ON_L2]
l2_to_l1_addresses[PLAY_ON_L2, PLAY]

But still on L1 side there is no update on mappings

Now bob gets minted PLAY_ON_L2 #50. He tries to bridge it back to alice

While alice tries to call withdrawTokens on L1

The mappings and request would be

req.collectionL1 = PLAY
req.collectionL2 = PLAY_ON_L2
l1Mapping = 0x00
l2Mapping = 0x00

If both req.collectionL1 and req.collectionL2 are non zero and if l1Mapping is not equal to req.collectionL1, verification would revert.

if (l2Req > 0 && l1Req > address(0)) {
if (l1Mapping != l1Req) {
revert InvalidCollectionL1Address();
} else if (l2Mapping != l2Req) {
revert InvalidCollectionL2Address();
} else {
// All addresses match, we don't need to deploy anything.
return l1Mapping;
}
}

This would lock the NFTs in L2 bridge

Similar issue is present in verify_collection_address in L2 bridge

if l2_req.is_zero() {
if l2_bridge.is_zero() {
// It's the first token of the collection to be bridged.
return ContractAddressZeroable::zero();
}
} else {
//@audit-issue L2 native tokens will be locked forever if bridged back as this will always revert
// L1 address is present, and L2 address too.
if l2_bridge != l2_req {
panic!("Invalid collection L2 address");
}
​
if l1_bridge != l1_req {
panic!("Invalid collection L1 address");
}
}

Impact

Users will loose NFTs forever

Tools Used

Manual review

Recommendations

Simplify verification logic. Remove strict checks if both l1 req and l2 req are present because they are set in bridge contracts which are trusted. Also consider using only one mapping in each bridge contract which just stores if a collection is already deployed

Updates

Lead Judging Commences

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

finding-first-bridgeof-a-collection-L1<->L2-do-not-sync-addresses

Likelyhood: High, any collections bridged, without bridge owner action, will be unable to bridge back. Impact: High, L2 -> L1 tokens will be stuck in the bridge. L1 -> L2 will need to ask for a cancellation.

Support

FAQs

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