ArkProject: NFT Bridge

Vulnerability Details

When Bridging tokens we are getting their URIs. We first try to get the baseURI, and if it contains value we just return it and ignore getting each NFT tokenURI separately.

TokenUtil.sol#L89-L92

If we check how the returned string value is compared we will find out that the returnedValue will always pass the check.

TokenUtil.sol#L158

• when we make the call we are storing the returned data at slot 0x00(returnOffset) with length 0x20(returnSize)

• Then, we store this value in returnValue

• Then, we compare it to be greater than 0

Now the issue is that the first 32 bytes in the returned data of a string variable is not its value nor its length it is the offset we are starting from.

To make it simpler if the string is < 32 bytes in length the returned bytes will be as follows:

Here is the returned bytes when the returned value is baseURI():

So when copying the first 32 bytes from slot 0x00 we are storing the offset not the length, which will make the value always be > 0, leading to pass the check even if baseUri() returned an empty string.

We should keep in mind that an empty string base URI is the default value for the base URI according to ERC721, so returning an empty string means it is not set, and we should not treat it as a valid baseURI.

Impact

Passing baseURI even if it returns an empty string (not present), and not getting each tokenURIs value separately, which is not how the function should work.

Tools Used

Manual Review and Foundry

Recommendations

Assign returnValue to be the string length value, not the offset loading the value from the offset.

This can be made by copying the first 0x40 bytes of the string, where the first 0x20 bytes will contain the offset and the second 0x20 byteswill contain the length (in normal Solidity encoding).

So we will store 0x40 bytes at memory slot 0x00 this is normal as 0x00 and 0x20 are not used, and then mload(0x20) to get the length.