Revert Lend - Silvermist's results

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L301-L309 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L312-L320 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L323-L326 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L329-L331

Vulnerability details

Impact

The V3Vault should be compliant with ERC4626 standart however maxDeposit, maxMint, maxWithdraw and maxRedeem are not fully up to EIP-4626's specification.

Proof of Concept

The EIP-4626 specifications states that maxDeposit and maxMint MUST factor in both global and user-specific limits.

Both maxDeposit and maxMint cover the case in which the value is more the globalLendLimit and return 0, but there is one case that is not covered.

Inside _deposit if the assets are more than the dailyLendIncreaseLimitLeft the transaction will revert. This should be taken into account in maxDeposit and maxMint and make them return 0 in that case.

 if (assets > dailyLendIncreaseLimitLeft) {
            revert DailyLendIncreaseLimit();
        } else {
            dailyLendIncreaseLimitLeft -= assets;
        }

Also, maxRedeem and maxWithdraw return the user's balance which is not correct. They MUST return the maximum amount of assets that could be transferred from owner through withdraw and not cause a revert

Inside _withdraw if the available is less than the assets the user wants to withdraw, the transaction will revert. In this case the user can only withdraw/redeem the available amount, not all of his amount as maxRedeem and maxRedeem return.

        (, uint256 available,) = _getAvailableBalance(newDebtExchangeRateX96, newLendExchangeRateX96);
        if (available < assets) {
            revert InsufficientLiquidity();
        }

Tools Used

Manual Review

Recommended Mitigation Steps

Rewrite maxDeposit as shown below, same logic can be used for maxMint with minimal differences.

function maxDeposit(address) external view override returns (uint256) {
        (, uint256 lendExchangeRateX96) = _calculateGlobalInterest();
        uint256 value = _convertToAssets(totalSupply(), lendExchangeRateX96, Math.Rounding.Up);
        if (value >= globalLendLimit) {
            return 0;
        } else {
            if (globalLendLimit - value > dailyLendIncreaseLimitLeft) {
                return 0;
            } else {
                return globalLendLimit - value;
            }
        }
    }

Rewrite maxRedeem as shown below, same logic can be used for maxWithdraw with minimal differences.

    function maxRedeem(address owner) external view override returns (uint256) {
        (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest();
        (, uint256 available,) = _getAvailableBalance(newDebtExchangeRateX96, newLendExchangeRateX96);
        if (available < balanceOf(owner)) {
            return available;
        } else {
            return balanceOf(owner);
        }
    }

Assessed type

ERC4626

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/transformers/AutoCompound.sol#L129 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/automators/Automator.sol#L148

Vulnerability details

Impact

In the Automator.sol the function _validateSwap uses UniswapV3.slot0 to get the value of sqrtPriceX96, however the sqrtPriceX96 gotten from Uniswap.slot0 is the most recent data point and can be manipulated easily via MEV bots & flashloans with sandwich attacks can cause lose of funds.

Proof of Concept

Automator.sol#_validateSwap

    function _validateSwap(
        bool swap0For1,
        uint256 amountIn,
        IUniswapV3Pool pool,
        uint32 twapPeriod,
        uint16 maxTickDifference,
        uint64 maxPriceDifferenceX64
    ) internal view returns (uint256 amountOutMin, int24 currentTick, uint160 sqrtPriceX96, uint256 priceX96) {
        // get current price and tick
        (sqrtPriceX96, currentTick,,,,,) = pool.slot0();


        // check if current tick not too far from TWAP
        if (!_hasMaxTWAPTickDifference(pool, twapPeriod, currentTick, maxTickDifference)) {
            revert TWAPCheckFailed();
        }


        // calculate min output price price and percentage
        priceX96 = FullMath.mulDiv(sqrtPriceX96, sqrtPriceX96, Q96);
        if (swap0For1) {
            amountOutMin = FullMath.mulDiv(amountIn * (Q64 - maxPriceDifferenceX64), priceX96, Q96 * Q64);
        } else {
            amountOutMin = FullMath.mulDiv(amountIn * (Q64 - maxPriceDifferenceX64), Q96, priceX96 * Q64);
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Use TWAP to get the value of sqrtPriceX96

Assessed type

MEV