Minting the registry NFT to a "user" always creates a reentrancy point

[JorgeAtPaladin]

https://github.com/etherisc/gif-next/blob/401ed87f073d09aaa7643a6a9fa76a9f195a6d82/contracts/registry/ChainNft.sol#L104

The mint function within the ChainNft uses _safeMint, this alternative to _mint creates a reentrancy vector on the receiver whenever this receiver is a smart contract.

Exploiters will always look for points in your code where such a mint is not the last operation in execution, and when it isn't, try to reenter in other points of the system to exploit it.

Potentially vulnerable spots include:

• Stake creation
• Policy creation
• Bundle creation
• ... other creations which move an NFT to the user

recommendation: The simplest solution is to simply not do the callback. This callback was introduced to avoid sending NFTs to eg. multisigs which didn't have the ability to transfer it again. I don't feel like that use case weights up to the refactoring challenges of making this mint reentrancy secure, which would be the actual solution. Hence simply removing the reentrancy altogether is the recommendation.

[doerfli]

@rapidddenis follow the recommendation. Please delete the interceptor calls during NFT minting and remove (I)TransferInterceptor.nftMint

[matthiaszimmermann]

fix together with #504

[rapidddenis]

Should the same consideration be taken for transfer callback / interception? But in this case interception is the must.

[doerfli]

Should the same consideration be taken for transfer callback / interception? But in this case interception is the must.

no, it relates only to minting.