search

code-423n4 / 2022-05-cally-findings

2 stars 0 forks source link

Cally Protocol Does Not Support Cryptopunk or Cryptokitties Tokens #207

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L198-L200

Vulnerability details

Impact

While the Cally.sol contract is compatible with standard ERC20 and ERC721 tokens, it is not able to handle popular non-standard ERC721 tokens such as cryptopunks. The typical transferFrom() call will fail as a result, and these users will likely opt to use another protocol which does support options on their specific NFT.

Recommended Mitigation Steps

NFTX protocol has implemented a way to handle the transfer of both standard and non-standard ERC721 tokens. The relevant implementation can be found here. The solution provided also utilises OpenZeppelin's safeTransferFrom() function on most transfers as this ensures the function reverts on failure.

outdoteth commented 2 years ago

Technically correct but we have no intention to support these tokens. users can use those tokens by getting a wrapped version of them that conforms to the erc721 spec

HardlyDifficult commented 2 years ago

This is a common issue when working with NFTs. Wrappers, native support, or simply not supporting these non-standard tokens are reasonable courses. Lowing to 1 (Low) and converting into a QA report for the warden.