Users can create vaults with a malicious _assetAddress

[code423n4]

Handle

jayjonah8

Vulnerability details

Impact

In NFTXVaultFactoryUpgradeable.sol a user can call the createVault() function passing in a malicious attacker controlled token as the _assetAddress. This will then create a vault setting the evil token address as the assetAddress in storage. Attacker controlled assets should never be allowed in a protocol as they can return arbitrary values when called upon performing other malicious tasks then whats expected.

Proof of Concept

https://github.com/code-423n4/2021-12-nftx/blob/main/nftx-protocol-v2/contracts/solidity/NFTXVaultFactoryUpgradeable.sol#L72

Tools Used

Manual code review

Recommended Mitigation Steps

Consider adding a mapping of approved tokens that can be passed in as an _assetAddress to the createVault() function to avoid malicious tokens.

[0xKiwi]

This issue is accurate but does not understand the implications of a malicious assetAddress.

This would mean the NFT itself has flaws built in, which would mean collectors of the NFT are collecting a malicious NFT, which doesn't really make sense.

Valid issue, but solution isn't possible and disagreeing with severity.

[dmvt]

I agree with everything the sponsor said and can't meaningfully expand on it. If the user has one of these NFTs, NFTX issues are the least of their concern.