Implement and use _safeMint rather than _mint

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/HolographERC721.sol#L814-L822
https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/HolographERC721.sol#L399-L411
https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/HolographERC721.sol#L508-L515

Vulnerability details

Impact

The custom implementation of a ERC721 it's not implement the function _safeMint
For example: The safeTransferFrom send and check the onERC721Received fallback

Proof of Concept

If the beneficiary of the mint it's a contract this asset could be stuck on it, if the contract not be prepare to receive assets

Tools Used

Review

Recommended Mitigation Steps

Implement the function _safeMint and use it like in OZ contracts

Note: be careful with the possible reentrancy attacks

[alexanderattar]

_safeMint is not required here

[rotcivegaf]

What happens if the beneficiary of the mint it's a contract who not be prepared to receive assets?
Or if the beneficiary expects the _checkOnERC721Received callback?

[alexanderattar]

The problem is that _safeMint opens up a security loophole where the receiving contract can introduce unexpected gas usages, malicious code, and reentrancy. Since mint is used in conjunction with bridge requests, this is a critical code path, and the costs of introducing potential attack vectors outweighs the concern of user error by minting to a contract that is not prepared to handle erc721 tokens.

References:

https://blocksecteam.medium.com/when-safemint-becomes-unsafe-lessons-from-the-hypebears-security-incident-2965209bda2a

https://samczsun.com/the-dangers-of-surprising-code/

[gzeoneth]

Consider with #422