search

code-423n4 / 2021-08-realitycards-findings

1 stars 0 forks source link

NFTHubs don't set `_baseUri` #59

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

Both RCNftHubL1 and RCNftHubL2 contracts do not set the _baseURI() which is the prefix of any tokenURI.

Impact

If all NFTs share a common prefix, it's more efficient to set it in the _baseURI() instead of setting and storing it individually in each setTokenURI call.

Recommended Mitigation Steps

If there's a common prefix, set it in the _baseURI().

Splidge commented 3 years ago

If all NFTs share a common prefix

They will not. Currently the URI is a URL pointing to our server, however this is a temporary measure until the UI is better able to read IPFS data. At that stage the URI will be the IPFS hash. It is tempting to assume the first two characters of the IPFS hash are constant (so we could use the _baseURI) as generally all the ones you'll see start Qm, I have looked into this because if we assume this then we can save the hash as bytes32 instead of a string and save storage slots. However those first two characters are not necessarily constant and so we can't save them as bytes32 or use the _baseURI feature.

0xean commented 3 years ago

closing as the base URI will not always be the same.