Submitted by carrotsmuggler, also found by falconhoof, PENGUN, SpicyMeatball, TheSchnilch, santipu_ (1, 2), 3docSec, ether_sky, and EV_om
The function totalBorrowedCredit is used to get an estimate of the amount of credit borrowed out by the system. The issue is that changes in creditMultiplier affect this value and can even lead to a revert due to underflow.
The function totalBorrowedCredit is defined as follows:
The first term is the total supply of the credit tokens, including rebasing rewards. The second part is the amount of credit tokens that can be redeemed, and is defined as shown below:
If creditMultiplier decreases due to a realized loss, the value returned by redeemableCredit goes up. If this value exceeds the targetTotalSupply() value, this function call will revert.
Assume a fresh market deployment. There are no loans at all. Alice now mints 1e18 credit tokens via the PSM, so the targetTotalSupply() is 1e18 and redeemableCredit is also 1e18. If the same situation is realized after the market has incurred a loss, the creditMultiplier will be less than 1e18, and the redeemableCredit will be greater than 1e18. This will cause the function to revert. A malicious borrower can also burn some of their credit tokens to help brick this function.
The totalBorrowedCredit function is used in debtCeiling calculations, and thus when it reverts it will also break term borrow operations, breaking functionality.
In this POC the following steps are demonstrated:
Put this in the ProfitManager.t.sol file:
The expectRevert in the end shows the revert. Consequences due to this failure is evident since this function is used in the debtCeiling calculations in lending terms.
Since this breaks lending terms, it is a critical issue.
Foundry
The totalBorrowedCredit function should never fail. Either cap it to 0 to prevent underflows. Or a better way is to track the total tokens minted by the PSM module with a variable which is incremented every mint and decremented every burn. This removes the dependence on creditMultiplier which can change over time even if PSM module users havent minted or burnt any excess tokens.
Under/Overflow
eswak (Ethereum Credit Guild) confirmed, but disagreed with severity and commented:
TrungOre (judge) decreased severity to Medium and commented:
