Use safeTransferFrom for ERC721 transfers #285
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
duplicate
This issue or pull request already exists
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Comments
code423n4
added
2 (Med Risk)
outdoteth
added
the
sponsor confirmed
|
use safeTransferFrom to prevent stuck NFTs: #38 |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Lines of code
Cally.sol#L295
Vulnerability details
Impact
The transferFrom function is used in the
createVault,withdrawand theexercisefunctions 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
exercisefunction, whenmsg.senderis 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
exercisefunction. This will revert if theonERC721Receivedfunction 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.
The text was updated successfully, but these errors were encountered: