search

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

2 stars 0 forks source link

Use safeTransferFrom for ERC721 transfers #285

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

Cally.sol#L295

Vulnerability details

Impact

The transferFrom function is used in the createVault, withdraw and the exercise functions to transfer the underlying ERC721 vault asset. transferFrom sends an ERC721 asset to a receiver regardless of whether it is able to receive it or not.

In the exercise function, when msg.sender is a contract, the transferFrom function could send the underlying ERC721 asset to a contract that cannot interact with an ERC721, effectively locking it.

Recommended Mitigation Steps

Consider using safeTransferFrom instead of transferFrom in the exercise function. This will revert if the onERC721Received function is not implemented in the receiving contract, not allowing an incompatible contract to exercise the option.

Incompatible contracts would still be able to buy options but would at least not waste more ETH when trying to exercise the option.

outdoteth commented 2 years ago

use safeTransferFrom to prevent stuck NFTs: https://github.com/code-423n4/2022-05-cally-findings/issues/38