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

Bridge does not have a function to upgrade the ERC721Bridgeable implementation

haxatron

Summary

Bridge does not have a function to upgrade the ERC721Bridgeable implementation

Vulnerability Details

When native L2 NFT is bridged, a new L1 NFT contract is deployed.

function deployERC721Bridgeable(
string memory name,
string memory symbol
)
public
returns (address)
{
...
return address(new ERC1967Proxy(impl, dataInit));
}

This contract is an ERC1967 proxy to allow for upgrading the ERC721Bridgeable implementation if needed. The ERC721Bridgeable implementation inherits UUPSProxied that can be upgraded via upgradeToAndCall(), and upgrading requires onlyOwner() permissions

/**
@notice Only owner should be able to upgrade.
*/
function _authorizeUpgrade(
address
)
internal
override
onlyOwner
{ }

TheERC721Bridgeable implementation transfers owner permission to the initializer, which will be the bridge

function initialize(
bytes calldata data
)
public
onlyInit
{
...
_transferOwnership(_msgSender());
}

However, since the bridge is the owner of the contract, the implementation cannot be upgraded, as there is no function in the bridge to do so.

Impact

The ERC721Bridgeable implementation cannot be upgraded as a result.

Tools Used

Manual Review

Recommendations

Add a function in the bridge to upgrade the ERC721Bridgeable implementation, similar to these functions for the Cairo bridge

impl BridgeCollectionAdminImpl of IStarklaneCollectionAdmin<ContractState> {
fn collection_upgrade(ref self: ContractState, collection: ContractAddress, class_hash: ClassHash) {
ensure_is_admin(@self);
IUpgradeableDispatcher { contract_address: collection }
.upgrade(class_hash);
}
​
// transfer owner of the given collection to the given address
fn collection_transfer_ownership(ref self: ContractState, collection: ContractAddress, new_owner: ContractAddress) {
ensure_is_admin(@self);
IOwnableDispatcher { contract_address: collection }
.transfer_ownership(new_owner);
}
}
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.

Appeal created

bladesec Auditor
10 months ago
n0kto Lead Judge
10 months ago
n0kto Lead Judge 9 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.