sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

0xReiAyanami - possible loss of funds because of miscalculation in liablitites #135

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

0xReiAyanami

medium

possible loss of funds because of miscalculation in liablitites

Summary

There is a possible loss of funds, because the borrower has the ability to call modify on an unhealthy position, which is not intended.

Vulnerability Detail

The Borrower.sol contract has a view function called getLiabilities which is used to check the current debt with the 2 Lender contracts involved. This view function is used in several other functions to be used as the base for the isHealthy check, which determines if the current position is still healthy (meaning, there is enough collateral and no bad debt)

The incentive for the Lender, to provide liquidity, is that he receives interest on the provided assets. This interest is accrued over time. To persist the currently accrued interest in the Lender.sol contract, there are the internal _load and _save functions. Which will update the stored Interest to the actual accrued interest. These are called whenever one of the external functions (e.g. repay, deposit, borrow, redeem), are called on the Lender. Additionally there is a separate external function called accrueInterest which is only calling _load and _save.

Also, there are 2 different view functions to receive the current debt of a specific borrower.

The beforementioned getLiabilities function of the Borrower.sol contract is using the latter one of these. Therefore it is not including the latest accrued interest.

For more frequently used markets, this is probably no issue as the lenders functions would be called regularly.

But, for less frequented markets there might arises a situation where the Lenders functions (including accrueInterest) are not called for a longer period of time, and therefore the stored borrowBalance is not updated with the latest interest.

This can result in a situation, where the Borrower.sol contract is still in a healthy state, but in regards to the ignored interest it would actually be unhealthy and should be liquidated.

In this state, the owner of the borrower.sol contract still can call modify and do potential harmful actions, like withdrawing assets up onto the limit (which is not respecting the latest interest as debt), which would basically steal a part of the interest from the lender.

Also, at this point the position could only be liquidated, after the interest was accrued in the lender.sol

Impact

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-L232

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

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

Tool used

Manual Review

Recommendation

Use the borrowBalancedStored instead or call the lenders accrueInterest before in important functions of borrower (modify, warn, liquidate)

Duplicate of #41

sherlock-admin2 commented 1 year ago

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

MohammedRizwan commented:

invalid issue recommendation as borrowBalancedStored() is already used in _getLiabilities()