Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol)
Description
Bug Description
Below I will explain an edge case in which a full liquidation (all debt to be repaid) will incorrectly calculate the amount to repay, resulting in an underflow error.
Below is an example state that I will use to explain the bug. For simple calculations, we will assume the violator has one debt position and one collateral position in the same asset and that the asset has a price of $1:
Below is the section of code that calculates the amount to repay and amount of collateral to seize (see line 461 below). Next, the violator's debt balance will be reduced by the repay amount, which can be greater than the debt balance, causing an underflow error (see line 478 below).
Since this is a full liquidation, the repayBorrowAmount will be set to the debt balance on line 212 above. On line 215 the repayBorrowAmount is converted into a dollar value and then the dollar value is converted to a collateral token amount, plus a liquidation bonus:
As we can see above, the calculated collateral to seize is greater than the available collateral balance for the violator. Therefore, the condition on line 226 in calcLiquidationAmounts will be true and the repayBorrowAmount will be re-calculated on line 228 in LiquidationLogic.sol, but this time it will be calculated based on the available collateral balance (total collateral balance is being seized):
As we can see above, the repayBorrowAmount is calculated the same way the collateral to seize was calculated, but this time the available collateral balance is being converted to an equivalent repay amount and then the liquidation bonus is added:
As we can see above, the re-calculated repayBorrowAmount is now greater than the actual debt balance. After this point, the repayBorrowAmount is used to decrement the violator's debt balance, which will result in an underflow:
The repayBorrowAmount should not be recalculated on line 228, since non-underwater positions (positions that will be liquidated for a profit) will have an overall collateral value greater than the debt value. Therefore, converting the collateral amount to an equivalent repay amount based on the collateral value will result in a repay value equivalent to the collateral value (collateral_value == repay_value). Additionally, the repay amount is then increased by the liquidation bonus. Therefore, as long as the debt position is not underwater (collateral_value >= debt_value), the recalculated repay amount will always result in a repay amount greater than the actual debt balance.
See Recommended Mitigation section below to observe an alternative way to readjust the repayBorrowAmount during these conditions.
Impact
Full liquidations will be blocked for certain unhealthy positions.
Necessary pre-conditions:
The debt position must have a high LTV (95% used in above example) so that the equivalent calculated collateral amount, when increased by the liquidation bonus, will exceed the available collateral balance. This means the collateral factor for the collateral asset would need to be 95% or the debt position remained unhealthy until the LTV reached 95%. Note that the LTV can be lower if the liquidation bonus is higher (800 used in above example) and vice versa.
loan is not underwater, i.e. collateral_value >= debt_value
Recommended Mitigation
On line 228 in LiquidationLogic.sol, the repayBorrowAmount should not be re-calculated the same way the collateral to be seized was calculated and instead should have simply been scaled down based on the ratio of available collateral and the calculated collateral to be seized. Going off our previous state, the calculation would look like so:
For simplicity, I chose to slightly modify a liquidation unit test for folks finance test suite in order to showcase this issue. The diff below shows what lines I altered and the expected output shows that the test fails due to an underflow error, as described in my report.
To run POC:
change the following files in the (/test/hub/LoanManager.test.ts)[https://github.com/Folks-Finance/folks-finance-xchain-contracts/blob/main/test/hub/LoanManager.test.ts] file
run test with npm run test ./test/hub/LoanManager.test.t
3) LoanManager (unit tests)
Liquidate
Should successfully liquidate variable borrow when seizing new borrow and collateral:
Error: VM Exception while processing transaction: reverted with panic code 0x11 (Arithmetic operation overflowed outside of an unchecked block)
