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

Inconsistency between `tokenURI` handling from L1 and L2 Sides

alqaqa

Vulnerability Details

In L1Bridge we are dealing with tokenURIs by only sending the baseURI if exists and if not we are sending each tokenURI separately.

TokenUtil.sol#L93-L103

// 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);
}

But if we checked how the case is handled when Bridging tokens from L2 to L1, we are not dealing with it that way. We are always getting tokenURIs separately. without trying to get the baseURI.

collection_manager.cairo#L33-L36

fn erc721_metadata( ... ) -> Option<ERC721Metadata> {
let erc721 = IERC721Dispatcher { contract_address };
​
❌️ let uris = match token_ids {
Option::Some(ids) => { ... },
Option::None => { ... }
};
​
Option::Some(
ERC721Metadata {
name: erc721.name(),
symbol: erc721.symbol(),
❌️ base_uri: "",
uris
}
)
}

This inconsistency in Logic implemented in both Bridges is not a good approach and will make transferring with only the base_uri, which is cheaper than filling an array of tokenURIs, not allowed from the L2 Bridge side.

Impact

Inability to Bridge tokens from L2 to L1 with base_uri support.

Tools Used

Manual Review

Recommendations

The same as we did in L1Bridge side. We will make a function call to get the base_uri first, and if existed we will just pass it with an empty token_uris array.

Updates

Lead Judging Commences

n0kto Lead Judge 9 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.