code-423n4 / 2021-05-visorfinance-findings

0 stars 0 forks source link

The function onERC721Received () allows writing duplicates in the array "nfts". Another functions dealing with this array do not expect duplicates met. #67

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

Sherlock

Vulnerability details

Impact

Duplicates can be written accidentally. If_removeNft() function is running, it will break when meeting the first match, not trying to remove other duplicates. Thus a caller should call removing a few times.

Proof of Concept

function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata) external override returns (bytes4) { _addNft(msg.sender, tokenId);

function _addNft(address nftContract, uint256 tokenId) internal { nfts.push( Nft({ tokenId: tokenId, nftContract: nftContract }) );

Tools Used

Hardhat

Recommended Mitigation Steps

In _addNft() to check if an inputted nft is existing in the "nfts" array. Do not push inputted nft if already added.

ghost commented 3 years ago

sponsor confirmed We will remedy this issue in our next version

ztcrypto commented 3 years ago

patch link