sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

SilentDefendersOfDeFi - Latest interest is not included in Liabilities, causes possible Loss of funds. #133

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

SilentDefendersOfDeFi

medium

Latest interest is not included in Liabilities, causes possible Loss of funds.

Summary

The borrower assesses their solvency or financial health by comparing their liabilities and assets. They obtain their liabilities by invoking the _getLiabilities() function from the lender. The issue with using borrowBalanceStored() instead of borrowBalance is that borrowBalanceStored() does not reflect the most recently accrued interest, and it only updates when another user triggers a function that involves loading and saving data on the lender. This situation permits the borrower to still call _modify(), even if they would otherwise be insolvent. For instance, they can still withdraw tokens or close a position.

Vulnerability Detail

Inside of borrower.sol _getLiabilities is used to calculate solvency.

function _getLiabilities() private view returns (uint256 amount0, uint256 amount1) {
        amount0 = LENDER0.borrowBalanceStored(address(this));
        amount1 = LENDER1.borrowBalanceStored(address(this));
    }

https://github.com/sherlock-audit/2023-10-aloe-Oot2k/blob/main/aloe-ii/core/src/Borrower.sol#L527-L530

Lender now has two functions to get the borrowBalance (these are located in ledger.sol, parent class):

**
     * @notice The amount of `asset` owed by `account` after accruing the latest interest. If one calls
     * `repay(borrowBalance(account), account)`, the `account` will be left with a borrow balance of 0.
     */
    function borrowBalance(address account) external view returns (uint256) {
        uint256 b = borrows[account];

        (Cache memory cache, , ) = _previewInterest(_getCache());
        unchecked {
            return b > 1 ? ((b - 1) * cache.borrowIndex).unsafeDivUp(BORROWS_SCALER) : 0;
        }
    }

    /// @notice The amount of `asset` owed by `account` before accruing the latest interest.
    function borrowBalanceStored(address account) external view returns (uint256) {
        uint256 b = borrows[account];

        unchecked {
            return b > 1 ? ((b - 1) * borrowIndex).unsafeDivUp(BORROWS_SCALER) : 0;
        }
    }

As we can see, borrowBalanceStored does not call _previewInterest(), so it returns the balance without latest intrest.

Impact

In smaller, less active markets, there is no assurance of frequent interest rate updates. This situation can be exploited by a borrower, especially if they hold a low-risk position that would only be liquidated through interest accrual. If the interest rate remains unaltered, the borrower can still execute various actions through _modify(). For instance, they can withdraw their tokens, effectively taking the unaccrued interest. (loss of funds)

Furthermore, there are other minor issues to consider, primarily related to potential fund shortages and bad debt when initiating a liquidation without prior execution of the accrueInterest function.

Code Snippet

https://github.com/sherlock-audit/2023-10-aloe-Oot2k/blob/main/aloe-ii/core/src/Borrower.sol#L527-L530

Tool used

Manual Review

Recommendation

Use borrowBalance instead of Balance Stored.

function _getLiabilities() private view returns (uint256 amount0, uint256 amount1) {
        amount0 = LENDER0.borrowBalance(address(this));
        amount1 = LENDER1.borrowBalance(address(this));
    }

Duplicate of #41

sherlock-admin2 commented 1 year ago

1 comment(s) were left on this issue during the judging contest.

MohammedRizwan commented:

valid issue