QA Report

[code423n4]

1. Duration issue

The user may accidently set the durationDays to be max of uint8 (255 days) and has his asset locked up for a very long time if the option's buyer does not exercise the option. I think that it would be in the best interest of the user if the locked-up period is fixed to a reasonable period like 30 days and the contract's owner should cleary communicate about the risk of user's asset unintentionally getting locked up if the option is not exercised.

2. Two-step authorization in transferOwnership()

Transfering ownership should be two step processes. The contract relies on openzeppelin's ownable which maybe problematic. Even when the function checks for zero address, it overlooks the case that the current owner may accidently input wrong address. To add additional security layer, the contract should add the acceptOwnership() that the nominated account will have to call

3. Checking whether the receiver is capable of holding ERC721

The contract usessafeTransfer() for ERC20 but uses transferFrom() for ERC721 in both exercise() and withdraw() which may lead to the loss of ERC721 if the receiving contract does not have onERC721Received(). To prevent this unintended circumstance, the contract should replace this function with safeTransferFrom() for ERC721 to check whether the receiving contract is IERC721Receiver.

[outdoteth]

this can be bumped to medium severity:

Checking whether the receiver is capable of holding ERC721; #38

[HardlyDifficult]

Moved receiver check to #317