UltraVault Manual Review Findings

UVT-01M: Insecure Integration of Parent Dependency

Type	Severity	Location
Logical Fault		UltraVault.sol:L171-L173

Description:

The UltraVault::beforeDeposit function is meant to integrate into the pre-deposit hook of the parent BaseControlledAsyncRedeem::_performDeposit function, however, in doing so it will cause all incoming deposits to be immediately diluted.

Specifically, the BaseControlledAsyncRedeem::_performDeposit function is invoked with an assets and shares pair reflecting the state of the vault before the fees have been captured.

As the fees result in an increase of the totalSupply of the token, the actual share-to-asset ratio is mutated within the UltraVault::beforeDeposit function even though the original BaseControlledAsyncRedeem::_performDeposit function would continue with the pre-fee share-to-asset ratio.

This results in a consistent over-dilution of new depositors which is incorrect.

Impact:

All new depositors are unfairly diluted as they will receive less shares than their assets would normally entail, causing existing shareholders to receive the diluted deposit's proceeds.

Example:

Copy
169/// @notice Collect fees before deposit
170/// @dev Expected to be overridden in inheriting contracts
171function beforeDeposit(address, uint256, uint256) internal virtual override {
172    _collectFees();
173}

Recommendation:

We advise fee collection to occur through overridden BaseControlledAsyncRedeem::_mintWithAsset and BaseControlledAsyncRedeem::_depositAsset functions, ensuring fees are captured prior to the preview functions and thus prior to the total supply measurements that deposit operations would rely on.

Alleviation (28f27853965de07fb79f4f2b5fed696d35120032):

The code was updated per our recommendation, overriding the relevant BaseControlledAsyncRedeem functions to ensure that fees are imposed correctly and securely.