code-423n4 / 2022-01-sandclock-findings

0 stars 0 forks source link

not compatible with ERC721 standard in Vaults/Claimer.sol #35

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

bugwriter001

Vulnerability details

Impact

in the function mint, it use the _mint method, which will not check the receiver whether it is compact with the ERC721 standard, namely if the address to is an contract, it should have the onERC721Received function, and returns the specific bytes4. to make sure it is ERC721 compatible.

function mint(
        address _to,
        uint256 _principal,
        uint256 _shares
    ) external onlyVault returns (uint256) {
        uint256 localTokenId = addressToTokenID[_to];

        if (localTokenId == 0) {
            _tokenIds.increment();
            localTokenId = _tokenIds.current();

            _mint(_to, localTokenId);
//------------
//better to be _safeMint(_to, localTokenId); 
        }

        claimers[localTokenId].totalShares += _shares;
        claimers[localTokenId].totalPrincipal += _principal;

        totalShares += _shares;

        return localTokenId;
    }

Proof of Concept

None

Tools Used

Recommended Mitigation Steps

naps62 commented 2 years ago

duplicate of #29