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

Improper Handling of URI Metadata in `ERC-721` Token Bridging Between L2->L1 leads to loss of `URI` on L1

Summary

The current implementation of token bridging mechanism between L1 and L2 does not account for URI preservation. Specifically, the improper handling of URI metadata for ERC-721 tokens during the withdrawal process on L1 can result in loss of metadata, leading to broken or incomplete tokens on L1 after they are withdrawn from L2.

Vulnerability Details

When a user deposits an ERC-721 token with a specific URI, the token's metadata, including its URI, is correctly managed on L2. The deposit_tokens function on L2 captures metadata for the ERC-721 tokens using erc721_metadata, which retrieves the name, symbol, base_uri, and uris such as the token's name, symbol, and URI and includes it in a Request struct that is sent to L1 when a token is deposited.

fn deposit_tokens(
ref self: ContractState,
salt: felt252,
collection_l2: ContractAddress,
owner_l1: EthAddress,
token_ids: Span<u256>,
use_withdraw_auto: bool,
use_deposit_burn_auto: bool,
) {
ensure_is_enabled(@self);
assert(!self.bridge_l1_address.read().is_zero(), 'Bridge is not open');
// ....
let erc721_metadata = erc721_metadata(collection_l2, Option::Some(token_ids));
let (name, symbol, base_uri, uris) = match erc721_metadata {
Option::Some(data) => (data.name, data.symbol, data.base_uri, data.uris),
Option::None => ("", "", "", array![].span())
};
// ...
let req = Request {
//...
name,
symbol,
base_uri,
ids: token_ids,
values: array![].span(),
uris,
new_owners: array![].span(),
};
// ...
}

However, during the withdrawal process on L1, the deserialization and minting functions do not account for URI preservation, resulting in a token on L1 that lacks its original metadata.

When a user initiates a withdrawal of the token from L2 back to L1 , the withdrawTokens function on L1 processes the withdrawal. Here, the withdrawal request is serialized into a uint256[] array. Then it is passed to the requestDeserialize function. This function correctly extracts the token ID, ownerL1, collectionL1, and other details, but fails to properly handle the URI metadata. While URIs are part of the Request object, there is no explicit check or validation for the presence and accuracy of URI data during deserialization.

function withdrawTokens(uint256[] calldata request) external payable returns (address) {
if (!_enabled) {
revert BridgeNotEnabledError();
}
// ...
Request memory req = Protocol.requestDeserialize(request, 0);
// ...
for (uint256 i = 0; i < req.tokenIds.length; i++) {
uint256 id = req.tokenIds[i];
bool wasEscrowed = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
if (!wasEscrowed) {
IERC721Bridgeable(collectionL1).mintFromBridge(req.ownerL1, id);
}
}
//...

Further, the withdrawTokens function checks if the token is present in the escrow. If the token is not found in escrow, it mints a new token on L1 using the mintFromBridge function:

if (!wasEscrowed) {
IERC721Bridgeable(collectionL1).mintFromBridge(req.ownerL1, id);
}

The mintFromBridge1 function is called without URI parameters. As a result, tokens minted on L1 lack their associated metadata, including the URI. This means a token that originally had metadata (e.g., an image or description linked through the URI) will be restored on L1 as a generic token without these details.

Example scenario:

  1. An NFT with a valid token id and URI metadata is deposited on L2.

  2. The withdrawal request includes the token ID and URI

  3. Upon withdrawal on L1, requestDeserialize correctly reads the token ID and other attributes but does not ensure that the URI is handled.

  4. If the token ID is not present in the escrow, the mintFromBridge function is called, minting a token on L1 without its original URI metadata.

  5. The resulting token on L1 appears incomplete, missing its associated image or other metadata initially tied to its URI.

Impact

Loss of token URI while bridging from L2->L1. NFTs rely heavily on metadata for their value proposition. Tokens that were deposited with specific metadata (e.g., images, descriptions) will lose this information upon withdrawal.

Tools Used

Manual Review

Recommendations

  1. Modify Minting Logic to Include URIs:

Update the mintFromBridge function in the IERC721Bridgeable interface to accept a URI parameter.

  1. Add Validation Checks in requestDeserialize:

Ensure that the deserialization process checks for the presence of URIs and that these URIs conform to expected standards. If missing or improperly formatted, revert the transaction.

Updates

Lead Judging Commences

n0kto Lead Judge about 1 year 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.