Description

The takeFees modifier in the ConcreteMultiStrategyVault contract calculates protocol fees using the feesUpdatedAt state variable and updates it after accruing fees:

modifier takeFees() {
    if (!paused()) {
        uint256 totalFee = accruedProtocolFee() + accruedPerformanceFee();
        uint256 shareValue = convertToAssets(1e18);
        uint256 _totalAssets = totalAssets();

        if (shareValue > highWaterMark) highWaterMark = shareValue;

        if (totalFee > 0 && _totalAssets > 0) {
            uint256 supply = totalSupply();
            uint256 feeInShare =
                supply == 0 ? totalFee : totalFee.mulDiv(supply, _totalAssets - totalFee, Math.Rounding.Floor);
            _mint(feeRecipient, feeInShare);
            feesUpdatedAt = block.timestamp;
        }
    }
    _;
}

The deposit and mint functions call _validateAndUpdateDepositTimestamps to initialize feesUpdatedAt to block.timestamp during the first deposit or mint:

function _validateAndUpdateDepositTimestamps(address receiver_) private {
    // [...]
    if (totalSupply() == 0) feesUpdatedAt = block.timestamp;
    // [...]
}

However, the takeFees modifier executes before _validateAndUpdateDepositTimestamps within both the deposit and mint functions:

function deposit(uint256 assets_, address receiver_)
    public
    override
    nonReentrant
    whenNotPaused
    takeFees
    returns (uint256 shares)
{
    _validateAndUpdateDepositTimestamps(receiver_);
    // [...]
}

function mint(uint256 shares_, address receiver_)
    public
    override
    nonReentrant
    whenNotPaused
    takeFees
    returns (uint256 assets)
{
    _validateAndUpdateDepositTimestamps(receiver_);
    // [...]
}

As a result, when takeFees runs for the first time, feesUpdatedAt remains uninitialized (i.e., zero). This causes accruedProtocolFee to treat the entire block.timestamp as elapsed time, resulting in protocol fees being calculated even though no time has actually elapsed for fee accrual. If the contract holds asset tokens at this point, the _mint(feeRecipient, feeInShare) call issues these unearned shares to the feeRecipient.

Impact

The feeRecipient can receive unearned shares during the first deposit or mint if the vault already holds asset tokens. This may result in an incorrect allocation of protocol fees and a dilution of other users' shares.

Recommendations

Consider refactoring _validateAndUpdateDepositTimestamps into a modifier, and ensure it executes before the takeFees modifier in the deposit and mint functions. This guarantees that feesUpdatedAt is properly initialized before any fee calculation or accrual logic runs.

Remediation

This issue has been acknowledged by Blueprint Finance, and a fix was implemented in commit 4f5b9785↗.