codyohl
commented
3 years ago fyi, @cemozerr @0xKiwi the recommendation of onlyEOA doesn't really work as a mitigation because it can still be done (and would be more likely to be done because it's gasless) via a flashbots bundle with an address and then a contract. Correct me if i'm wrong?
see https://github.com/flashbots/pm/issues/24 for why flashbots could offer this type of account abstraction.
Handle
0xRajeev
Vulnerability details
Impact
If we assume that certain NFTs in a vault over time will have different market demand/price then the users will try to redeem those specific NFTs. Even if direct redeems are disabled to prevent such a scenario to default to returning randomized NFTs, a user can brute-forced this on-chain randomization (using nonce + blockhash) by repeatedly trying to redeem/swap from a contract, checking the NFT IDs returned from the function and reverting the transaction if those are not the NFT IDs of specific interest.
The impact will be a subversion of the randomization goal to return random NFTs which cannot be specified by the user.
A similar exploit happened recently with Meebit NFTs https://twitter.com/sillytuna/status/1391013965170454540
Proof of Concept
https://github.com/code-423n4/2021-05-nftx/blob/f6d793c136d110774de259d9f3b25d003c4f8098/nftx-protocol-v2/contracts/solidity/NFTXVaultUpgradeable.sol#L376
https://github.com/code-423n4/2021-05-nftx/blob/f6d793c136d110774de259d9f3b25d003c4f8098/nftx-protocol-v2/contracts/solidity/NFTXVaultUpgradeable.sol#L413-L427
Tools Used
Manual Analysis
Recommended Mitigation Steps
Consider onlyEOA for redeem/swap operations to prevent brute-forcing via contracts. Alternatively, make the user commit to pseudo-random IDs before revealing them.