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

L1->L2 loss of individual token URIs when _baseUri() or baseUri() returns an empty string

crunter

Summary

When bridging tokens from L1 to L2, TokenUtil::erc721Metadata uses the _callBaseUri function to determine whether a base URI for the collection already exists; if so, the individual token URIs are ignored. However, _callBaseUri will succeed even if it reads an empty string as the collection's base URI, resulting in a loss of the individual token URIs.

Vulnerability Details

ERC721 NFT contracts may have a public baseUri function that returns the base URI of the collection, set via _setBaseURI. The returned string is empty by default (see openzeppelin-contracts/contracts/token/ERC721
/ERC721.sol).

When baseUri (or _baseUri) exists and is called by TokenUtil::_callBaseUri(), the call will succeed even for the empty string case, and the function will return parameters that TokenUtil::erc721Metadata will take as valid:

function erc721Metadata(address collection, uint256[] memory tokenIds)
internal
view
returns (string memory, string memory, string memory, string[] memory)
{
bool supportsMetadata = ERC165Checker.supportsInterface(collection, type(IERC721Metadata).interfaceId);
​
if (!supportsMetadata) {
return ("", "", "", new string[](0));
}
​
IERC721Metadata c = IERC721Metadata(collection);
@> // 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) { // @audit success will be true even if the string returned by baseUri is empty
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);
}
}

When baseUri function exists and returns an empty string, the empy string will be read by the _callBaseUri function and the parameters values will be

success == true

_baseUri == ""

The empty string will be used as a valid base URI and will cause individual token URIs to be ignored.

Proof of concept

The fact that _callBaseUri succeeds when baseUri function returns an empty string is easily seen by just slightly modifying the WithUnderscoreBaseUri and/or WithBaseUri contracts in TokenUtils.t.sol file as follows:

contract WithUnderscoreBaseUri is IUnderscoreBaseUri {
function _baseUri() external view returns (string memory) {
- return "_baseURI here";
+ return ""; // empty string
}
}
​
contract WithBaseUri is IBaseUri {
function baseUri() external view returns (string memory) {
- return "_baseURI here";
+ return ""; // empty string
}
}

then, execute the test_with_baseuri test. It will succeed.

Impact

Impact: High

Likelihood: Medium

This problem arises every time a collection with baseUri or _baseUri function returning an empty string is bridged, resulting in loss of the individual token URIs.

Tools Used

Manual Review

Recommendations

To fix this issue, it is recommended to add an additional check for the string returned by _callBaseUri in the TokenUtil::erc721Metadata function:

function erc721Metadata(
address collection,
uint256[] memory tokenIds
)
internal
view
returns (string memory, string memory, string memory, string[] memory)
{
bool supportsMetadata = ERC165Checker.supportsInterface(
collection,
type(IERC721Metadata).interfaceId
);
if (!supportsMetadata) {
return ("", "", "", new string[](0));
}
​
IERC721Metadata c = IERC721Metadata(collection);
// 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) {
+ if (success && keccak256(abi.encodePacked(_baseUri)) != keccak256(abi.encodePacked(""))) {
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);
}
}

A second valid solution is to use the individual URIs even if a base URI exists:

function erc721Metadata(
address collection,
uint256[] memory tokenIds
)
internal
view
returns (string memory, string memory, string memory, string[] memory)
{
bool supportsMetadata = ERC165Checker.supportsInterface(
collection,
type(IERC721Metadata).interfaceId
);
if (!supportsMetadata) {
return ("", "", "", new string[](0));
}
​
IERC721Metadata c = IERC721Metadata(collection);
- // 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);
+ return (c.name(), c.symbol(), _baseUri, URIs);
- }
}
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-empty-baseURI-do-not-use-tokenURI

Likelyhood: Low, baseURI is not in the ERC721 standard. Impact: Low, function won’t work when it has to. But URI can be set later.

Appeal created

crunter Submitter
about 1 year ago
n0kto Lead Judge
about 1 year ago
n0kto Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-empty-baseURI-do-not-use-tokenURI

Likelyhood: Low, baseURI is not in the ERC721 standard. Impact: Low, function won’t work when it has to. But URI can be set later.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.