`UniV3Vault` does not distribute fee earning to depositor

[code423n4]

Handle

gzeon

Vulnerability details

Impact

UniV3Vault does not distribute fee earning to depositor. Fee from Uniswap V3 LP is collected by permissioned collectEarnings function that allow owner to send fee to arbitrary address. Since LP fees are the only reason for anyone to LP (the expected return of pure LP without fee is -ve verus hodl), there are no reason for the fee collected to be considered separately from the tvl. And since UniV3Vault does not consider fee earning when calculating tvl, this will lead to depositors miss out the fee entirely anyway.

Proof of Concept

https://github.com/code-423n4/2021-12-mellow/blob/6679e2dd118b33481ee81ad013ece4ea723327b5/mellow-vaults/contracts/UniV3Vault.sol#L100

     function tvl() public view override returns (uint256[] memory tokenAmounts) {
        tokenAmounts = new uint256[](_vaultTokens.length);
        if (uniV3Nft == 0)
            return tokenAmounts;
        (
            , , , , , 
            int24 tickLower, 
            int24 tickUpper, 
            uint128 liquidity,
            , , ,
        ) = _positionManager().positions(uniV3Nft);
        (uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
        uint160 sqrtPriceAX96 = TickMath.getSqrtRatioAtTick(tickLower);
        uint160 sqrtPriceBX96 = TickMath.getSqrtRatioAtTick(tickUpper);
        (uint256 amount0, uint256 amount1) = LiquidityAmounts.getAmountsForLiquidity(
            sqrtPriceX96,
            sqrtPriceAX96,
            sqrtPriceBX96,
            liquidity
        );
        tokenAmounts[0] = amount0;
        tokenAmounts[1] = amount1;
    }

collectEarnings allow owner to send fee to arbitary address https://github.com/code-423n4/2021-12-mellow/blob/6679e2dd118b33481ee81ad013ece4ea723327b5/mellow-vaults/contracts/UniV3Vault.sol#L84

require(owner == msg.sender || _isValidPullDestination(to), ExceptionsLibrary.VALID_PULL_DESTINATION);

and not called anywhere https://github.com/code-423n4/2021-12-mellow/blob/6679e2dd118b33481ee81ad013ece4ea723327b5/mellow-vaults/contracts/UniV3Vault.sol#L80

$grep -n -R -F -i "collectEarnings" ./contracts
contracts/UniV3Vault.sol:80:    function collectEarnings(address to) external nonReentrant returns (uint256[] memory collectedEarnings) {

Recommended Mitigation Steps

Make collectEarnings permissionless, incentivize it or otherwise make sure it is called regularly. Make sure collectEarnings send the reward back to the vault but not anywhere else. Consider fee earning when calculating tvl.

[MihanixA]

collectEarnings is designed to be called by strategist in this version or by automated strategy in future versions.

[0xleastwood]

As collectEarnings is intended to be called continuously in future versions, I will not rate this issue as high severity. However, the current version may be impacted if the strategist does not routinely call collectEarnings. There is no loss of funds or impact on the protocol, but the current design can be misleading to depositors. Marking as low severity.