ArkProject: NFT Bridge

Github links

solidity

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Bridge.sol#L368-L375
https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/token/CollectionManager.sol#L138-L141

cairo

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/starknet/src/bridge.cairo#L360-L364
https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/starknet/src/token/collection_manager.cairo#L189-L196

Summary

Whitelist can't be disabled because the setL1L2CollectionMapping and set_l1_l2_collection_mapping will always act as a form of whitelist. This means all nfts using the bridge will always be permissioned.

Vulnerability Details

Assuming the bridge is permissionless, and you own everai nft#1337 and you decide to bridge from l1 -> l2, it would work fine. However, if you try to bridge back from l2 -> l1, this check in the _verifyRequestAddresses will fail

the l1 bridge contract will never know the l2 address of a native l1 nft. The same is true for native l2 nfts when you try to do l2 -> l1 -> l2 bridge transactions. The l2 bridge will not know the l1 address and the verify_collection_address function will fail. This is unless of course the setL1L2CollectionMapping (Bridge.sol) function or set_l1_l2_collection_mapping (bridge.cairo) function is called by an admin.

this means that even if the enable_whitelist_function(false) is called, the set collection mappings functions would still necessarily act as a form of whitelist.

Impact

Bridge will always need to whitelist nfts

Recommendations

During a bridge transaction from l1 to l2 and vice versa, we can trust that the req.collectionL1 and req.collectionL2are correct because the bridges only communicate with each other. so there should be no check that l1Mapping == l1Req or l2Mapping == l2Req in both bridge contracts.

Then the setL1L2CollectionMapping and set_l1_l2_collection_mapping can be removed.