Revert Lend - BowTiedOriole's results

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L1246-L1268

Vulnerability details

Impact

Per the whitepaper, the max daily debt/lend increase should be capped at 10% of the current lend/debt amount. The V3Vault._resetDaily(Lend/Debt)IncreaseLimit functions multiply the current amount by 110% instead of 10%.

function _resetDailyLendIncreaseLimit(uint256 newLendExchangeRateX96, bool force) internal {
    // daily lend limit reset handling
    uint256 time = block.timestamp / 1 days;
    if (force || time > dailyLendIncreaseLimitLastReset) {
        // @audit-medium Is multiplying by 110% instead of 10%
        uint256 lendIncreaseLimit = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up)
            * (Q32 + MAX_DAILY_LEND_INCREASE_X32) / Q32;
        dailyLendIncreaseLimitLeft =
            dailyLendIncreaseLimitMin > lendIncreaseLimit ? dailyLendIncreaseLimitMin : lendIncreaseLimit;
        dailyLendIncreaseLimitLastReset = time;
    }
}

This means that if 100,000 USDC has been lended to the vault, the dailyLendIncreaseLimitLeft will be set to 110,000 USDC instead of 10,000 USDC.

Proof of Concept

function testDailyIncreaseLimitBug() external {
    // Adjust limits to show vulnerability
    uint256 amount = 10e6;
    vault.setLimits(0, 1000e6, 1000e6, amount, amount);

    // lend 10 USDC to the vault
    assertEq(vault.dailyLendIncreaseLimitLeft(), amount);
    _deposit(amount, WHALE_ACCOUNT);

    // increase time and deposit 0 to update dailyLendIncreaseLimitLeft
    vm.warp(block.timestamp + 1 days);
    _deposit(0, WHALE_ACCOUNT);

    // This should instead be the max between 10e6 (min increase) and 1e6 (10%)
    assertEq(vault.dailyLendIncreaseLimitLeft(), 10_999_999);
}

Tools Used

Manual Review

Recommended Mitigation Steps

Do not add Q32 to the MAX_DAILY_LEND_INCREASE_X32 or MAX_DAILY_DEBT_INCREASE_X32

function _resetDailyLendIncreaseLimit(uint256 newLendExchangeRateX96, bool force) internal {
    // daily lend limit reset handling
    uint256 time = block.timestamp / 1 days;
    if (force || time > dailyLendIncreaseLimitLastReset) {
        uint256 lendIncreaseLimit = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up)
-           * (Q32 + MAX_DAILY_LEND_INCREASE_X32) / Q32;
+           * uint256(MAX_DAILY_LEND_INCREASE_X32) / Q32;
        dailyLendIncreaseLimitLeft =
            dailyLendIncreaseLimitMin > lendIncreaseLimit ? dailyLendIncreaseLimitMin : lendIncreaseLimit;
        dailyLendIncreaseLimitLastReset = time;
    }
}

function _resetDailyDebtIncreaseLimit(uint256 newLendExchangeRateX96, bool force) internal {
    // daily debt limit reset handling
    uint256 time = block.timestamp / 1 days;
    if (force || time > dailyDebtIncreaseLimitLastReset) {
        uint256 debtIncreaseLimit = _convertToAssets(totalSupply(), newLendExchangeRateX96, Math.Rounding.Up)
-           * (Q32 + MAX_DAILY_DEBT_INCREASE_X32) / Q32;
+           * uint256(MAX_DAILY_DEBT_INCREASE_X32) / Q32;
        dailyDebtIncreaseLimitLeft =
            dailyDebtIncreaseLimitMin > debtIncreaseLimit ? dailyDebtIncreaseLimitMin : debtIncreaseLimit;
        dailyDebtIncreaseLimitLastReset = time;
    }
}

Assessed type

Math

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L372-L375 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L378-L381 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L1175-L1179

Vulnerability details

Impact

The lendExchangeRateX96 is dependent upon the utilization rate of debt in the Vault.

(, uint256 available,) = _getAvailableBalance(oldDebtExchangeRateX96, oldLendExchangeRateX96);

uint256 debt = _convertToAssets(debtSharesTotal, oldDebtExchangeRateX96, Math.Rounding.Up);

(uint256 borrowRateX96, uint256 supplyRateX96) = interestRateModel.getRatesPerSecondX96(available, debt);

Due to unfavorable timing, this rate can change after a user has called redeem or withdraw but prior to the transaction being confirmed. Since the Vault is lacking any slippage checks, the user may receive fewer assets or burn more shares than expected.

Proof of Concept

Withdraw and redeem functions contain no slippage checks.

function withdraw(uint256 assets, address receiver, address owner) external override returns (uint256) {
    (, uint256 shares) = _withdraw(receiver, owner, assets, false);
    return shares;
}

function redeem(uint256 shares, address receiver, address owner) external override returns (uint256) {
    (uint256 assets,) = _withdraw(receiver, owner, shares, true);
    return assets;
}

Tools Used

Manual Review

Recommended Mitigation Steps

Both withdraw/redeem functions should include slippage protection parameters provided by the users (either minimum amount out for redeem function or maximum shares in for withdraw function).

Assessed type

Context