Vulnerability Details

When Bridging NFT tokens we need to handle their URIs, according to there Base URI if existed, or each NFT tokenURI separately if not.

TokenUtil.sol#L89-L92

Now if we check how getting baseURI is done, we will find that the logic contains a log of mistakes.

TokenUtil.sol#L150-L155

First, we are calling _baseUri(), then if the call fails we call baseUri() function to get the baseURI, but calling these two functions will always fail (100%) because of the following reasons.

1. Calling internal functions will always fail

_baseUri() is not a public function, it is an internal function in ERC721, so calling this will always revert, i.e failed.

2. Incorrect function naming will lead to calling different function signature

The signature for base URI is _baseURI by making the three letters UpperCase, but in the code we are making just the U is in upper case and r, i are small letters, this will result in a totally different signature.

3. There is no function signature named baseURI() or baseUri() in NFT contract

The third thing is that baseUri/baseURI() is not even exist in ERC721, and if we checked the modified version for ERC721 made by the protocol ERC721Bridgeable, we will find that it is also not implementing any public function to retrieve that baseURI separately.

So by collecting all these things together, we can conclude that retrieving the baseURI will always fail.

This mean we will always end up looping through all NFT items we want to Bridge, getting their URIs to Bridge a large number of tokens, even if the collection supports baseURI.

TokenUtil.sol#L97-L103

Impacts

• 100% DoS for the function used to retrieve baseURI

• The cost for Bridging large tokens will increase significantly when Bridging Large NFTs amount as we will loop through all there URIs to get them.

• payload size will increase a lot without knowing, which can even make NFT end up locking in the Bridge Due to a payload size limit in Starknet (This is a known issue, but we are highlighting that the payload will increase without knowing. When Bridging 20 NFTs for example supports base URI, the user thinks that tokenURIs array is 0, but it will actually be 20 which will make the payload size increase significantly)

Tools Used

Manual Review

Recommendations

1. Remove calling _baseUri() as it is an internal function which will always fail.

2. Change the function name to call from baseUri() to baseURI().

3. In ERC721Bridgeable make a public function named baseURI() that returns the base URI value.

NOTE: We planned to group this bug into only one issue, although it contains three different root causes as we illustrated. But we think that all of them relate to the same thing. If the Judger decides to separate into different issues, Please consider this issue to be dup for all separated issues (as it mentioned all root causes).