NFTBridge
60,000 USDC
View results
Submission Details
Severity: low
Valid

Token URIs imported incorrectly for certains types of ERC721 collections

Summary

NFTs from ERC721 collections that use custom logic for token URIs combining both the base URI and a token specific uri (such as NFTs based on the OZ ERC721URIStorage) will have the wrong URI when bridged from a source contract on Ethereum to a bridgable contract on starknet, breaking their behaviour in specialized wallets or dapps and compromizing their value in general.

Vulnerability Details

The logic around importing Token URIs from a source ERC721 contract on Ethereum to a bridgable contract on Starknet can be broken to three parts:

1. Encoding URI information into the bridging message on Ethereum

The logic around Token URI retrieval from L1 to L2 is located in the TokenUtils.erc721metadata function:

// How the URI must be handled.
// if a base URI is already present, we ignore individual URI
// else, each token URI must be bridged and then the owner of the collection
// can decide what to do
(bool success, string memory _baseUri) = _callBaseUri(collection);
if (success) {
return (c.name(), c.symbol(), _baseUri, new string[](0));
}
else {
string[] memory URIs = new string[](tokenIds.length);
for (uint256 i = 0; i < tokenIds.length; i++) {
URIs[i] = c.tokenURI(tokenIds[i]);
}
return (c.name(), c.symbol(), "", URIs);
}

As can be seen in the code and comment the logic is: If a non-empty baseUri can be retrieved from the collection, include only the base uri in the message (without individual NFT URIs). Otherwise, retrieve each bridged token's URI without including the base URI in the message.

2. Extracting and Using the URI information when minting the "mirror" tokens on Starknet

On the starknet side, when a collection from L1 is first bridged in, a bridgable NFT contract is created, with the baseURI that came with the message:

let l2_addr_from_deploy = deploy_erc721_bridgeable(
self.erc721_bridgeable_class.read(),
salt,
req.name.clone(),
req.symbol.clone(),
req.base_uri.clone(),
starknet::get_contract_address(),
);

When a specific Token ID is first bridged, the same token ID is minted on the bridgable contract. If the message includes token URIs, the token is minted with its specific URI:

if (req.uris.len() != 0) {
let token_uri = req.uris[i];
IERC721BridgeableDispatcher { contract_address: collection_l2 }
.mint_from_bridge_uri(to, token_id, token_uri.clone());
} else {
IERC721BridgeableDispatcher { contract_address: collection_l2 }
.mint_from_bridge(to, token_id);
}

3. Starknet side erc721_bridgable tokenURI logic

On starknet the logic for retrieving a token URI from a bridgable contract is:

fn token_uri(self: @ContractState, token_id: u256) -> ByteArray {
let uri = self.token_uris.read(token_id);
if uri != Default::default() {
uri
} else {
self.erc721.token_uri(token_id)
}
}

In other words, if the token was minted with a specific uri, return it, otherwise fallback to the base erc721 implementation, which is: concatenate the token ID to the base uri (e.g. www.baseuri.com/tokenId)

The core of the issue is that the logic described above does not work with NFT collections with custom token URI logic combining both the base URI and a token specific part. An example is the OZ ERC721URIStorage contract where a tokenUri is based on a baseUri concatenated with a Token-specific part saved per-token in storage.

Ark's logic will bridge only the baseUri is this case. This means that when calling the erc721_bridgable token_uri function, the base implementation of OZ erc721.token_uri ( returning www.baseuri.com/tokenId) is called, returning a faulty Uri.

Root Cause

Incorrect Token URI handling when bridging from Ethereum:

(bool success, string memory _baseUri) = _callBaseUri(collection);
if (success) {
return (c.name(), c.symbol(), _baseUri, new string[](0));
}

(for collections with custom tokenUri logic, should send specific token URIs instead)

NFTs that use the described custom logic (that will not work with Ark) are very common and include prominent collections such as Bored Ape Yacht Club, Art Blocks, Rarible NFTs and many others.

Impact

Many prominent Ethereum NFT collections use custom Uri logic that won't work with Ark's bridge ( Bored Ape Yacht Club, Art Blocks, Rarible NFTs and many others).

These collections will have incorrect Token Uris when bridged through ARK, breaking the behaviour of apps/dapps using them (e.g. NFT wallets and apps relying on token uri for presentation, gaming NFTs functioning incorrectly, social media integratrions etc.)

As a whole this would seriously compromize Ark's bridge value/brand and prevent many collections from using it.

Tools Used

Manual Review, Foundry, snFoundry

Recommended Mitigation

Change the _whiteList map to map to a struct with additional information. Add to that struct a field specifying a preferred tokenURI import method (to be determined as part of the whitelisting process). Use that field to apply different logics to how token URIs are imported depending on the case (i.e. standard NFTs - send only baseUri, custom NFTs - send individual token Uris)

Updates

Lead Judging Commences

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

finding-baseUri-selector-instead-of-baseURI

Likelyhood: Medium, no token using OZ version 2.X and 3.X will work. Impact: Low, Valid standard token won’t be mint with the URI but owner can use ERC721UriImpl function on the deployed token.

Support

FAQs

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