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

There is no way to modify the created NFT collection on L1.

said

Summary

When an NFT collection is bridged from L2 to L1 and there is no corresponding L1 collection, a new ERC721 contract will be deployed. However, there is no way to modify the NFT collection contract due to the lack of functionality within the L1 bridge contract.

Vulnerability Details

When new collection is created on the L1 side, it will set bridge as the owner of the ERC721 contract.

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/token/CollectionManager.sol#L49-L70

function _deployERC721Bridgeable(
string memory name,
string memory symbol,
snaddress collectionL2,
uint256 reqHash
)
internal
returns (address)
{
address proxy = Deployer.deployERC721Bridgeable(name, symbol);
_l1ToL2Addresses[proxy] = collectionL2;
_l2ToL1Addresses[collectionL2] = proxy;
​
emit CollectionDeployedFromL2(
reqHash,
block.timestamp,
proxy,
snaddress.unwrap(collectionL2)
);
​
return proxy;
}
​

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/token/Deployer.sol#L23-L38

function deployERC721Bridgeable(
string memory name,
string memory symbol
)
public
returns (address)
{
address impl = address(new ERC721Bridgeable());
bytes memory dataInit = abi.encodeWithSelector(
ERC721Bridgeable.initialize.selector,
abi.encode(name, symbol)
);
​
return address(new ERC1967Proxy(impl, dataInit));
}
​

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/token/ERC721Bridgeable.sol#L37-L49

function initialize(
bytes calldata data
)
public
onlyInit
{
(string memory n, string memory s) = abi.decode(data, (string, string));
​
_name = n;
_symbol = s;
​
_transferOwnership(_msgSender());
}

However, within the L1 Bridge contract, there is no functionality that allows the admin to interact with the created ERC721. This restriction prevents modification of the ERC721 contract if needed.

Impact

This restriction prevents modification of the ERC721 contract if needed. For instance, if there is a metadata modification on L2, the corresponding L1 NFTs cannot be adjusted to match it, potentially causing the L1 NFTs to lose their value.

Tools Used

Manual review

Recommendations

Add functionality to the L1 Bridge so it can interact with the created ERC721 collections.

Updates

Lead Judging Commences

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

finding-no-transferOwnership-or-upgrade-for-collections-in-CollectionManager

Likelyhood/Impact: High, it will never (until an upgrade) be able to update or transfer the ownership of any collections created on L1.

Support

FAQs

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