toshii - Protocol understates the value of user's collateral, which can lead to issues including premature liquidation

[sherlock-admin]

toshii

high

Protocol understates the value of user's collateral, which can lead to issues including premature liquidation

Summary

The protocol does not correctly calculate the value of a user's underlying collateral, which can lead to multiple issues, with the most prevalent being premature liquidation

Vulnerability Detail

Although the undervaluing of a user's collateral can lead to many issue with the protocol functionality, the most prevalent is the potential for premature liquidations, which will lead to unfair loss of user funds. Fundamentally the issue is that the value of a user's collateral includes the interest owed to that user for their collateral. However, the protocol does not correctly handle incorporating this interest. Let's see how.

When a user is being liquidated, another user will call liquidate. In this call, interest is first accrued to marketBorrow and marketCollateral. Then, a check is done to see if the borrower is liquidatable:

_accrueInterest(marketBorrow, mBorrow);
_accrueInterest(marketCollateral, mCollateral);

// Check if the borrower is actually liquidatable.
require(_isLiquidatable(borrower), "borrower not liquidatable");

The _isLiquidatable function loops over all the markets that the borrower has collateral in, and uses that to calculate the total value of the user's collateral:

uint256 liquidationCollateralValue;
...
for (uint256 i = 0; i < userEnteredMarkets.length; i++) {
	...
	if (supplyBalance > 0 && liquidationThreshold > 0) {
		uint256 exchangeRate = _getExchangeRate(m);
		liquidationCollateralValue += (supplyBalance * exchangeRate * assetPrice * liquidationThreshold) / 1e36 / FACTOR_SCALE;
	}
	...
}

The exchangeRate value effectively converts a given amount of ibTokens that the user has to the underlying tokens. However, this value is only correct for a given market directly following a call to accrue interest for that market. The main issue arises because this function does not accrue interest to all of the markets that a user has collateral in prior to calculating the value of the user's tokens. This means that their total collateral value can be severely undervalued, especially if they have a lot of unaccounted for interest in markets that the liquidator is not liquidating collateral from.

Impact

The protocol is in many cases undervaluing the collateral of a given user, which can lead to premature liquidation of that user and ultimately unfair loss of funds, even if they have enough collateral to cover their loan

Code Snippet

Referenced lines of code:
https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/IronBank.sol#L499
https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/IronBank.sol#L1082-L1086
https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/IronBank.sol#L810
https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/IronBank.sol#L919

Tool used

Manual Review

Recommendation

In the _isLiquidatable function, accrue interest to all markets which a user has collateral in prior to calculating the exchangeRate for those markets.

[0xToshii]

Escalate for 10 USD
Thank you for your review. As I laid out in this issue, there is a fundamental problem with the current implementation in that a user's collateral is being undervalued, especially if they have positions across many different markets. This can lead to irreversible damage, such as early liquidation. This is a known and well-accepted issue. You can see an example here: sherlock-audit/2023-02-blueberry-judging#140.

[sherlock-admin]

Escalate for 10 USD
Thank you for your review. As I laid out in this issue, there is a fundamental problem with the current implementation in that a user's collateral is being undervalued, especially if they have positions across many different markets. This can lead to irreversible damage, such as early liquidation. This is a known and well-accepted issue. You can see an example here: sherlock-audit/2023-02-blueberry-judging#140.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[0xffff11]

Agree with escalation.

[0xffff11]

Given the new input and as signaled by sponsor in issue: #499 I agree with the non-duplication.

I see the case for both medium and low here too given the sponsors comments. I think this issue and its comments, regardless of severity, should get the same severity than #360 and its duplicates.

[hrishibhat]

Result:
Low
Unique
#499 (comment)

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• 0xtoshii: rejected