sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

0x007 - _getLiabilities uses borrowBalanceStored instead of borrowBalance #110

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0x007

high

_getLiabilities uses borrowBalanceStored instead of borrowBalance

Summary

Borrower._getLiabilities uses borrowBalanceStored. This is inappropriate cause it doesn't reflect the current liability of the borrower.

Vulnerability Detail

Lender provides borrowBalanceStored and borrowBalance. The difference is that borrowBalance returns a value as if interest has been accrued. While borrowBalanceStored uses the stored borrowIndex which could be outdated.

// https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/Ledger.sol#L212-L233

/**
* @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;
    }
}

When it's time to repay, interest would be accrued. In other words, borrowBalance would be used and it is in fact written in the docs

/**
* @notice Reduces `beneficiary`'s debt by `units`, assuming someone has pre-paid `amount` of `asset`. To repay
* all debt for some account, call `repay(borrowBalance(account), account)`.
* @dev To avoid frontrunning, `amount` should be pre-paid in the same transaction as the `repay` call.
* @custom:example ```solidity
*   PERMIT2.permitTransferFrom(
*     permitMsg,
*     IPermit2.SignatureTransferDetails({to: address(lender), requestedAmount: amount}),
*     msg.sender,
*     signature
*   );
*   lender.repay(amount, beneficiary)
* ```
*/

But the liability used in Borrower is the stored, not current

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

Impact

The returned liabilities are used in warn, liquidate and modify. The incorrect value could cause a lot of bad behaviors such as

Code Snippet

https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/Borrower.sol#L527-L530 https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/Ledger.sol#L212-L233 https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/Lender.sol#L244

Tool used

Manual Review

Recommendation

Use borrowBalance in _getLiabilities

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