Use `_safeMint` instead of `_mint`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L303-L308

Vulnerability details

If fillOrder is called by a contract that expect onERC721Received to be called when receiving NFT, it could potentially result in irregular state for the receiver contract.

PuttyV2.sol#L303-308

_mint(order.maker, uint256(orderHash));
...
_mint(msg.sender, positionId);

The current implementation uses _mint instead of _safeMint. As ERC721 transfers within the contract use the safeTransferFrom variant, it would be better to keep it consistent and use the safe variant, so as not to confuse contract developers that integrate Putty.

Proof of Concept

• Alice wants to make a contract that could interact with Putty.
• Alice expects that Putty conforms to ERC721TokenReceiver standard after seeing the ERC721.safeTransferFrom implementation, so she relies on onERC721Received to update the contract's state when minting the option NFT.
• Alice tried to fulfill an order via the contract and lost her funds as the contract's state is not updated properly after mint, making it unable to call withdraw or exercise.

Recommended Mitigation Steps

Add _safeMint function in PuttyV2Nft.sol and use it instead of _mint in fillOrder().

[outdoteth]

Duplicate: Contracts that can’t handle ERC721 tokens will lose their Putty ERC721 position tokens: https://github.com/code-423n4/2022-06-putty-findings/issues/327