QA Report

[code423n4]

QA

1. QA
   1. L-Missing SetMaxFee
   2. L-Can send ETH more than buyOption premium required
   3. N-Consider use ERC721 SafeTransferFrom instead of transferFrom
   4. L-Can create vault with EOA address as ERC20 token address

L-Missing SetMaxFee

If setFee set higher than 1e18. exercise will underflow.

During live production, it is possible to accidentally set fee higher than it suppose to be.

Cap max fee 5%,10%,20%(20e17) would prevent any accident or malicious owner handling.

L-Can send ETH more than buyOption premium required

Link.
require(msg.value >= premium) should be require(msg.value == premium) to prevent user send too much eth. Only beneficiary benefit from this. Contract should protect user from these nuance mistakes.

N-Consider use ERC721 SafeTransferFrom instead of transferFrom

withdraw and exercise already implement check and effect parttern. There is no risk of reentrancy with ERC721 safeTransferFrom. This provides service for user who use special NFT wallet. Or simply prevent user from withdraw NFT to unsupported contract.

L-Can create vault with EOA address as ERC20 token address

CreateVault does not check if token have extCodeSize > 0 or not. User can create vault with EOA address as ERC20 token address (tokenType = ERC721 throw error). Interact vault with buyOption and exercise work as normal. This happens due to external raw call to EOA always return success. Interface ERC721 wrap address call does not.

Contract should prevent create accident vault with non-contract address.

[outdoteth]

these can be bumped to medium severity:
L-Missing SetMaxFee; #48
L-Can send ETH more than buyOption premium required: #84
N-Consider use ERC721 SafeTransferFrom instead of transferFrom; #38

this can be bumped to high risk:
L-Can create vault with EOA address as ERC20 token address: #225

[HardlyDifficult]

Although this report touches on some important changes, it's lacking detail about the potential risk / abuse that can result. Per the C4 guidance "part of auditing is demonstrating proper theory of how an issue could be exploited". Going to score this as a high quality QA report instead.

[HickupHH3]

For L-Can send ETH more than buyOption premium required: "to prevent user send too much eth. Only beneficiary benefit from this. " seems sufficient to me for explaining the issue. How is the description different from #245 for instance? It is also inconsistent with the decision given for #56: "the more important point is they identified an issue"