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

Token URI data is sent when bridging a collection from Starknet to Ethereum but never updated on the Ethereum side

nirohgo

Summary

The Starknet-side bridge allways sends token URI data when bridging to Ethereum, but the Ethereum-side ERC721Bridgable contract does not use this data and will always return an empty string for any tokenId when queried with tokenURI(uint256 tokenId). This results in both a loss for bridge users (who pay a redundant fee for the transfered URI data) and in rendering the Ethereum-side bridged tokens useless for most common NFT applications.

Vulnerability Details

The token bridging process from Starknet to Ethereum involves the following steps with regards to handling token URIs

  1. When depositing tokens on starknet, specific token URIs are always included in the message while base URI is never included:

//return value code from erc721_metadata in collection_manager.cairo
Option::Some(
ERC721Metadata {
name: erc721.name(),
symbol: erc721.symbol(),
base_uri: "",
uris
}
)
​
//code from bridge deposit_tokens
let req = Request {
header: compute_request_header_v1(ctype, use_deposit_burn_auto, use_withdraw_auto),
hash: compute_request_hash(salt, collection_l2, owner_l1, token_ids),
collection_l1,
collection_l2,
owner_l1,
owner_l2: from,
name,
symbol,
base_uri,
ids: token_ids,
values: array![].span(),
/*note*/uris,
new_owners: array![].span(),
};

  1. On the Ethereum side, when the message is consumed through WidthdrawTokens, if the bridgable collection does not exist, it is created, but the base URI is never set ( see here ).

  2. If a specific token ID is bridged to Ethereum for the first time, is it minted on the Ethereum-side bridgable contract, but the token's URI (sent with the message) is never used ( see here ).

  3. Any call to an ERC721Bridgable.tokenUri(tokenId) on Ethereum falls back to the base OZ ERC721 implementation, that returns an empty string if the _baseUri() is empty:

/**
* @dev See {IERC721Metadata-tokenURI}.
*/
function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
_requireMinted(tokenId);
​
string memory baseURI = _baseURI();
return bytes(baseURI).length > 0 ? string(abi.encodePacked(baseURI, tokenId.toString())) : "";
}

  1. As a result, any bridged Starknet collection will have no URI data for any Token, ispite of this data being sent with every bridge message.

Impact

  1. Ethereum-side bridged NFTs are useless for many NFT related applications and uses that relies on the URI for NFT presentation (social media apps, NFT wallets, Marketplaces, gaming apps NFTs etc.)

  2. Users pay an extra gas fee (both on Starknet when sending the message and on Ethereum when withdrawing it) for the URI data sent, inspite of this data never being used.

Tools Used

Manual Review, Foundry, snFoundry

Recommended Mitigation

  1. Implement per-token URIs on the ERC721Bridgable contract and set the sent URIs when a token is first bridged to the Ethereum side.

  2. Until such imlpementation is available, avoid sending URI information from the Starknet-side bridge.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

invalid-NFT-minted-without-baseURI-URIs-or-bridge-with-no-metadata

URI is not lost on the origin chain and it can be modified with `ERC721UriImpl`. As explained in the TODO  below, that’s a design choice and it will be implemented as a future feature. https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Bridge.sol#L206 `ERC721Bridgable` is out of scope.

Support

FAQs

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