code423n4 commented 3 years ago

Handle

Sherlock

Vulnerability details

Impact

An attacker can deal direct economic damage to the owner/delegate spending some gas to spam the array of "nfts" with different values. It will be more costly to remove these nfts one-by-one, transaction-by-transaction. Also, it makes other functions dealing with "nfts" array more costly.

Proof of Concept

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

Tools Used

Hardhat

Recommended Mitigation Steps

To check in modifiers of ERC721Received () that only the owner or delegates can invoke this function. Revert if someone else calls.

ghost commented 3 years ago

sponsor confirmed We will be adding a check so that only ERC721 sent from owner account will add to array.

ztcrypto commented 3 years ago

patch link

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