NFT's can be frozen at exercise()

[code423n4]

Lines of code

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

Vulnerability details

Impact

exercise() function is called by a call option buyer to exercise the option. When this happens below code transfers the underlying NFT to the exerciser:
// transfer the NFTs or ERC20s to the exerciser
vault.tokenType == TokenType.ERC721
? ERC721(vault.token).transferFrom(address(this), msg.sender, vault.tokenIdOrAmount)
: ERC20(vault.token).safeTransfer(msg.sender, vault.tokenIdOrAmount);
However, if the receiver address is a contract address which do not have ERC721 support, then the NFT will be frozen in the contract.

Proof of Concept

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

Tools Used

Manual analysis

Recommended Mitigation Steps

I suggest to use safeTransferFrom, so that if the receiver can not receive the NFT, exercise would revert.
// transfer the NFTs or ERC20s to the exerciser
vault.tokenType == TokenType.ERC721
? ERC721(vault.token).safeTransferFrom(address(this), msg.sender, vault.tokenIdOrAmount)
: ERC20(vault.token).safeTransfer(msg.sender, vault.tokenIdOrAmount);

[outdoteth]

use safeTransferFrom to prevent stuck NFTs: #38