sherlock-audit / 2023-04-ajna-judging

4 stars 3 forks source link

Chinmay - The kick function uses outdated LUP to update Interest State #108

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Chinmay

medium

The kick function uses outdated LUP to update Interest State

Summary

The kick function in Pool.sol updates interest rates etc. via the _updateInterestState function using the LUP from the KickResult struct. But unlike the other functions in Pool.sol that use _updateInterestState function, kick uses an outdated value of LUP for the calculations.

Vulnerability Detail

Every major function in Pool.sol calls PoolCommons.updateInterestState via the _updateInterestState function at the end of their logic to update the Interest Rates, EMA values, debt etc. The concern here is that in the kick function, the new LUP value needs to be passed on to the _updateInterestState function in order to use it to update EMA values etc. An example is Pool.sol : addQuoteToken => LenderActions.sol : addQuoteToken. We can see at LenderActions.sol#L205 that the LUP is recalculated if the deposit has changed the relevant dynamics of top buckets used for LUP. This newLUP is then passed on to the _updateInterestState function at Pool.sol#L169.

Similarly for the drawDebt functionality we can see that ERC20Pool : drawDebt => BorrowerActions.sol : drawDebt call flow leads to BorrowerActions.sol#L217 where first poolDebt is updated and the new LUP is then calculated based on that.

The concern here is that the kick function does update the pool Debt but does not recalculate the LUP considering the newly accrued debt (which in the case of kick is the 90 days of interest added to the kicked borrower's debt ), and passes on the old value of LUP based on unupdated LUP to the call to _updateInterestState function.

This _updateInterestState function called at Pool.sol#L309 then uses this outdated LUP to calculate the lupt0Debt and update interestParams_.lupt0Debt with this wrong value. This leads to wrong calculations. See PoolCommons.sol#L114 and L204. Though like other functions, poolDebt has been updated at Pool.sol#L293

Impact

Outdated value of LUP passed on to the _updateInterestState function leads to wrong values downstream.

Also, this creates an inconsistency in poolDebt and corresponding LUP since this debt is 90 days of interest penalty on a kicked borrower(which means this is a permanent debt change).

Code Snippet

https://github.com/sherlock-audit/2023-04-ajna/blob/e2439305cc093204a0d927aac19d898f4a0edb3d/ajna-core/src/base/Pool.sol#L310

Tool used

Manual Review

Recommendation

Recalculate the LUP in kick just like how its done in other functions mentioned. This will remove the accounting inconsistency between poolDebt and corresponding LUP as well as prevent wrong downstream calculations.

Duplicate of #107