sherlock-audit / 2024-06-velocimeter-judging

11 stars 7 forks source link

Hajime - `ownerOf()` implementation non-ERC721 compliant #573

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

Hajime

Medium

ownerOf() implementation non-ERC721 compliant

Summary

The ERC721 standard requires that ownership queries for NFTs assigned to the zero address must revert, indicating invalid/non-existent NFT. However, the VotingEscrow.ownerOf() implementation returns address(0) instead, violating this standard. This deviation risks causing functional and security issue.

Vulnerability Detail

VotingEscrow.ownerOf() implementation returns address(0) instead, violating this standard

    function ownerOf(uint _tokenId) public view returns (address) {
        return idToOwner[_tokenId];
    }

For a non-existent _tokenId, this implementation will return address(0) instead of reverting. This behavior is not compliant with the ERC721 specification.

Impact

This non-compliance can lead to serious compatibility issues with external integrations that rely on standard ERC721 behavior. Applications expecting a revert when querying ownership of an invalid NFT might instead receive a non-reverting response, potentially resulting in erroneous processing or security vulnerabilities in syste

Code Snippet

https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/VotingEscrow.sol#L190-L192

Tool used

Manual Review

Recommendation

use ERC721 standard

Duplicate of #492