Wrong debt accouting in liquidatePosition()
Closed this issue · 0 comments
Lines of code
Vulnerability details
Impact
In liquidatePosition(), deltaDebt is calculated from repayAmount by reducing the penalty and this "deltaDebt" is the value that is then used for debt reduction accounting.
The current logic for deltaDebt accounting is wrong because it leaves out the penalty while writing off debt from a position storage.
As a result of this, the line deltaDebt == maxRepayment here will never be reached even on full liquidation (as deltaDebt < repaid amount). Even though we want the liquidator to only pay a discounted amount, for accounting purposes we need to use the actual debt value being written off.
Since repayAmount is user-controlled, liquidator can try to input a slightly higher amount such that after penalty reduction the deltaDebt output becomes equal to maxRepayment and succeed for full liquidation, but this is not possible because of the constraint on takeCollateral here : takeCollateral check
Because of this bug, the debt of a position even on full liquidation will never be set to zero (as logic never reaches deltaDebt == maxRepayment). This non-zero position.debt value will keep stranded in the storage and lead to incorrect profit accounting for this repay call, but a bigger impact is that it will more frequently revert if the resultant debt amount becomes too small (< debtFloor, which is likely as penalty is a small value) => This will DOS full liquidation of a undercollateralized position.
Proof of Concept
Tools Used
Manual review
Recommended Mitigation Steps
We need to write off the complete debt to zero when a position's collateral becomes zero, ie. after a full liquidation, a position's storage should not have any debt left. Solution depends on if the protocol wants to allow partial liquidations, but liquidatePositionBadDebt() correctly handles this case and sets position debt == 0 as full liquidation is enforced in that case.
Assessed type
DoS