Upgraded Q -> 2 from #664 [1677633674294]

[c4-judge]

Judge has assessed an item in Issue #664 as 2 risk. The relevant finding follows:

2- Vault fees can be set greater than 1e18 in the initialize function : The Vaut contract implements 4 types of fees (deposit, withdrawal, management, performance) collected when the user deposits or withdraw tokens, those fees are calculated in basis point (BPS) where 1e18 is equivalent to 100% of the given amount.

The function proposeFees is used to set the value for new fees and it ensures that their respective values are always less than 1e18, but when the vault is initialized during deployment the initialize function (of the Vault contract) does not check the value of the fees set and thus it allows fees that are greater than 1e18.

This does not have a big impact as users can't call the deposit or mint functions (as they both underflow in this case) but this will block the vault until the owner proposes new fees and then you need to wait until the quitPeriod period has passed to update the fees and open the vault.

Risk : Low Proof of Concept The issue occurs in the initialize which does not contain any check for the value of fees :

File: vault/Vault.sol Line 57-98

function initialize( IERC20 asset, IERC4626 adapter, VaultFees calldata fees, address feeRecipient, address owner ) external initializer { __ERC20init( string.concat( "Popcorn ", IERC20Metadata(address(asset)).name(), " Vault" ), string.concat("pop-", IERC20Metadata(address(asset_)).symbol()) ); __Owned_init(owner);

if (address(asset_) == address(0)) revert InvalidAsset();
if (address(asset_) != adapter_.asset()) revert InvalidAdapter();

asset = asset_;
adapter = adapter_;

asset.approve(address(adapter_), type(uint256).max);

_decimals = IERC20Metadata(address(asset_)).decimals();

INITIAL_CHAIN_ID = block.chainid;
INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator();

feesUpdatedAt = block.timestamp;
fees = fees_;

if (feeRecipient_ == address(0)) revert InvalidFeeRecipient();
feeRecipient = feeRecipient_;

contractName = keccak256(
    abi.encodePacked("Popcorn", name(), block.timestamp, "Vault")
);

emit VaultInitialized(contractName, address(asset));

} As you can see the fees are set immediately without any check on their values.

Mitigation Add a check in the initialize function of the Vault contract to ensure that the fees are always less than 1e18.

[c4-judge]

dmvt marked the issue as duplicate of #396

[c4-judge]

dmvt marked the issue as satisfactory