Omniscia Evergon Labs Audit

ERC721DataManager Manual Review Findings

ERC721DataManager Manual Review Findings

ER2-01M: Deviation of Specification

Description:

The NonFungibleTokenDO contract interaction performed via the ERC721DataManager::_ownerOf function will not fail if no address is identified and will instead yield zero. This contradicts the EIP-721 standard which defines that the ERC721DataManager::ownerOf function must throw if an NFT is owned by the zero address as such situations are considered invalid.

Impact:

A minor deviation from the EIP-721 specification has been observed in the ERC721DataManager in relation to ownership queries.

Example:

contracts/dataManagers/ERC721/ERC721DataManager.sol
83/// @inheritdoc IERC721
84function ownerOf(uint256 tokenId) public view returns (address) {
85    return _ownerOf(tokenId);
86}
87

88function _checkMinter() internal view override {
89    _checkOwner();
90}
91

92function _writeTransfer(address from, address to, uint256 tokenId) internal virtual override {
93    if (to == address(0)) {
94        dataIndex.write(nonFungibleTokenDO, _datapoint, INonFungibleTokenOperations.burn.selector, abi.encode(from, tokenId));
95    } else if (from == address(0)) {
96        dataIndex.write(nonFungibleTokenDO, _datapoint, INonFungibleTokenOperations.mint.selector, abi.encode(to, tokenId));
97    } else {
98        dataIndex.write(nonFungibleTokenDO, _datapoint, INonFungibleTokenOperations.transferFrom.selector, abi.encode(from, to, tokenId));
99    }
100}
101

102function _ownerOf(uint256 tokenId) internal view virtual override returns (address) {
103    return abi.decode(nonFungibleTokenDO.read(_datapoint, INonFungibleTokenOperations.ownerOf.selector, abi.encode(tokenId)), (address));
104}

Recommendation:

We advise a similar check to the ERC721DataManager::balanceOf function to be applied, ensuring that the ERC721DataManager is properly compliant with the EIP-721 standard it is meant to implement.

Alleviation (c6b23c23d8bcd8cce85049ad959cbd711a37126b):

The code of the ERC721DataManager::ownerOf implementation was updated to revert if an owner is not found, alleviating this exhibit.

Prev

ERC721Burnable.sol (ERL-M)

Next

FractionERC1155DataManager.sol (FEM-M)

On this page