Front-running setFees() could avoid fees

[code423n4]

Handle

0xRajeev

Vulnerability details

Impact

setVaultFeatures() and setFees() are two separate privileged functions. Users could front-run setFees() immediately after vault is enabled in setVaultFeatures() to mint (and possibly redeem/directRedeem/swap) many tokens. The fees for mint/redeem/directRedeem/swap are not initialized so are 0 by default. This leads to loss of fee revenue.

Proof of Concept

https://github.com/code-423n4/2021-05-nftx/blob/f6d793c136d110774de259d9f3b25d003c4f8098/nftx-protocol-v2/contracts/solidity/NFTXVaultUpgradeable.sol#L45-L48

https://github.com/code-423n4/2021-05-nftx/blob/f6d793c136d110774de259d9f3b25d003c4f8098/nftx-protocol-v2/contracts/solidity/NFTXVaultUpgradeable.sol#L141-L158

https://github.com/code-423n4/2021-05-nftx/blob/f6d793c136d110774de259d9f3b25d003c4f8098/nftx-protocol-v2/contracts/solidity/NFTXVaultUpgradeable.sol#L123-L139

Tools Used

Manual Analysis

Recommended Mitigation Steps

Set defaults at initialization or combine this with setVaultFeatures() for atomically enabling functions and setting their fees in the same transaction.