ArkProject: NFT Bridge

Summary

ERC721Bridgeable contracts which are deployed either by L1 bridge or collection owner cannot be upgraded further due to ownership to upgrade is being to L1 bridge contract

Vulnerability Details

When users bridge tokens from L2, if there is no correponding collection on L1 side, the L1 bridge would deploy an ERC721Bridgable contract as a collection.

ERC721Bridgable inherits a UUPSOwnableProxied to support upgradablity. UUPSOwnableProxied inherits a Ownable contract to manage ownership. So the owner is responsible to handle the upgrades. Here the issue is the owner is same for both operations i.e upgrade and mintFromBridge. And when the collection is deployed, the ownership is transferred to msg.sender i.e to the L1 bridge contract.

The L1 bridge contract can never upgrade the collection as it doesn't support any neccesary functions to do that. This leaves deployed collection not upgradable further.

Aditionally even if the collection is deployed by owner manually first, collection ownership must be transfered to the bridge for the bridge to use it and call mintFromBridge. So once ownership is transfered to bridge, again the contract will be not upgradable anymore.

So this completely renders implementation of upgragability logic for ERC721Bridgable contract useless.

Impact

ERC721Bridgeable collections on L1 (either deployed by bridge or collection owners) cannot be upgraded.

Tools Used

Manual review

Recommendations

1. Have a different field for collection owner which should be set while deployed. Collection owner should be responsible for upgrades.

2. Don't add the bridge as owner but just intialise the bridge address and check caller address matches when mintFromBridge is called