Potential reentrancy in the swapTo function of NFTXVaultUpgradeable

Handle shw Vulnerability details Impact The swapTo function in the NFTXVaultUpgradeable contract should use the modifier nonReentrant to prevent reentrancy, which could happen when a user receives an NFT and calls the swapTo function again in the onERC721Received or onERC1155Received functions he...

getRandomTokenIdFromFund yields wrong probabilities for ERC1155

Handle @cmichelio Vulnerability details Vulnerability Details NFTXVaultUpgradeable.getRandomTokenIdFromFund does not work with ERC1155 as it does not take the deposited quantity1155 into account. Impact Assume tokenId0 has a count of 100, and tokenId1 has a count of 1. Then getRandomId would have...

getPseudoRand can be predicted

Handle @cmichelio Vulnerability details Vulnerability Details The NFTXVaultUpgradeable.getPseudoRand is not really random and can be predicted. It's also easy to make sure that one gets the correct token by having a smart contract simulate the randomness logic before the call to redeem / swap...

Manager can grief with fees

Handle @cmichelio Vulnerability details Vulnerability Details The fees in NFTXVaultUpgradeable can be set arbitrarily high no restriction in setFees. Impact The manager can frontrun mints and set a huge fee for example fee = base which transfers user's NFTs to the vault but doesn't mint any pool...

The direct redeem fee can be circumvented

Handle janbro Vulnerability details Summary The direct redeem fee can be circumvented Risk Rating Medium Vulnerability Details Since the random NFT is determined in the same transaction a payment or swap is being executed, a malicious actor can revert a transaction if they did not get the NFT the...

Missing access restriction on NFTXVaultUpgradeable.finalizeFund

Handle @cmichelio Vulnerability details Vulnerability Details Missing access restriction on NFTXVaultUpgradeable.finalizeFund. Impact Anyone can lock out the manager by calling finalizeFund which sets the manager to 0. This griefing attack can prevent managers from setting correct fees, vault...

Missing allValidNFTs and afterRedeemHook with swapTo?

Handle gpersoon Vulnerability details Impact The function swapTo of NFTXVaultUpgradeable.sol is kind of a combination of mintTo and redeemTo the code looks very similar to a combination of mintTo and redeemTo. Before receiveNFTs I would expect a call to allValidNFTs, like in mintTo. This is to ma...

getRandomTokenIdFromFund not really random

Handle gpersoon Vulnerability details Impact The function getRandomTokenIdFromFund of NFTXVaultUpgradeable.sol is not really random, as noted in the name of the function getPseudoRand. The value of the blockhashblock.number - 1 is fully determined for al the transactions in the same block. The...