This repository has been archived by the owner on Dec 24, 2023. It is now read-only.
Chinmay - Wrong Inflator used in calculating HTP to determine accrualIndex in accrueInterest #111
Labels
Medium
A valid Medium severity issue
Reward
A payout will be made for this issue
Sponsor Confirmed
The sponsor acknowledged this issue is valid
Will Fix
The sponsor confirmed this issue will be fixed
Chinmay
medium
Wrong Inflator used in calculating HTP to determine accrualIndex in accrueInterest
Summary
When accruing Interest, the interest is added according to the deposits in all buckets upto the lower of LUP and HTP prices' buckets. But HTP is wrongly calculated in the
accrueInterest
function.Vulnerability Detail
All major functions in
Pool.sol
use_accrueInterest
which makes a call toPoolCommons.accrueInterest
to accrue the interest for all deposits above the minimum of LUP and HTP prices, which means upto the lower of theLupIndex
andHTPIndex
because indexes move in the opposite direction of prices. Here inaccrueInterest
function, theaccrualIndex
is calculated as the higher of LUPIndex and HTPIndex to make sure interest is calculated for all deserving deposits abovemin(HTP price, LUP price)
ie.max(LUP index, HTP index)
.But the
accrualIndex
has been implemented incorrectly. The HTP is calculated using thenewInflator
which incorporates the newly accrued interest into the HTP calculation, whereas the LUP is calculated with old values. TheaccrualIndex
is set tomax(LUP index, HTP index)
. then.Assume that the LUP price > HTP price. So, LUP index < HTP index. Hence, for the
interestEarningDeposit
all the deposits above the HTP index will be considered. But the value of HTP index is wrong now because it is calculated using new Inflator which means that the new Interest has been added in calculation of HTP already and thus the derived HTP index will be lower in value(which means upper in the bucket system). Assume that still after this LUP index < HTP indexNow since the old LUP index and new HTP index is used in the
max(LUP index, HTP index)
function, and LUP index is still < HTP index (ie. LUP price > HTP price) thus the deposits that were between the old HTP index and the new HTP index have been left out of theinterestEarningDeposit
calculation.I talked to the developers about this discrepancy between new HTP and old LUP being used, and they said
"I can see the argument in favor of using the prior inflator here to be totally precise."
Also, one of them said,
"It should be computed using the debt prior to the interest accrual itself, as it's determining the the minimum amount of deposit onto which that interest would be applied"
This means that the htp index has been underestimated because it has been made lower(ie. upper in the bucket system) and thus the deposits that lie between the old HTP index and new HTP index have not been considered for calculating
interestEarningDeposit
when they should have been considered because before the interest accrual itself, those deposits were under the deservingmax(LUP index, HTP index)
bracket.This means
interestEarningDeposit
has been underestimated and later calculations at PoolCommons.sol#L253 for lender Factor have become wrong.Impact
This is a logic mistake and leads to wrong values for the lenderFactor.
Code Snippet
https://github.com/sherlock-audit/2023-04-ajna/blob/e2439305cc093204a0d927aac19d898f4a0edb3d/ajna-core/src/libraries/external/PoolCommons.sol#L232
Tool used
Manual Review
Recommendation
Calculate htp using the old inflator to correctly include all deposits that were deserving to get into
interestEarningDeposit
calculation.The text was updated successfully, but these errors were encountered: