Description
DebtToken.sol's mint() function performs double accounting and mints more debtTokens than expected, increasing the borrow liability of the borrower. For example, consider how an ideal situation should look like:
Alice borrows 500 crvUSD. At this point, both balanceOf(alice) and scaledBalanceOf(alice) ---> which is equivalent to super.balanceOf(alice) are 500.
Usage index is 1 right now (i.e. 1 RAY or 100%)
During 100 days, interest accrues & usage index increases to 1.02 i.e. a 2% interest or 2% * 500 = 10
The amount owed can be seen by balanceOf(alice) as 500 + 10 = 510 (correctly)
The scaledBalanceOf(alice) shows 500 (correctly)
If she were to settle her loan, she is expected to pay 510
What if she borrows an additional 100 and immediately wants to settle the loan? It should be 510 + 100 = 610 as no interest accrued on the just borrowed 100. So balanceOf(alice) should show 610 while scaledBalanceOf(alice) should show 500 + 100/1.02 = ~ 598.
However mint() shows these values as approximately 620 and 608 respectively due to incorrect calculations. (Refer the Mitigation section of the report to see the incorrect code lines).
Impact
Inflated amount of debtTokens are minted & wrong figures are reported throughout the protocol. Functions like _repay() and finalizeLiquidation() in LendingPool.sol call DebtToken.sol's burn() function which makes use of balanceOf() which would now return a higher debt figure and can cause sooner than expected liquidations.
In reality, most borrow related accounting is broken due to this.
Proof of Concept
Add this inside LendingPool.test.js and run to see the following output:
it("demonstrates DebtToken mint bug", async function () {
const tokenId2 = 2;
const amountToPay2 = ethers.parseEther("1000");
await raacHousePrices.setOracle(owner.address);
await raacHousePrices.setHousePrice(tokenId2, amountToPay2);
await token.mint(user1.address, amountToPay2);
await token.connect(user1).approve(raacNFT.target, amountToPay2);
await raacNFT.connect(user1).mint(tokenId2, amountToPay2);
await raacNFT.connect(user1).approve(lendingPool.target, tokenId2);
await lendingPool.connect(user1).depositNFT(tokenId2);
const borrowAmount = ethers.parseEther("500");
await lendingPool.connect(user1).borrow(borrowAmount);
const usageIndex1 = await lendingPool.getNormalizedDebt();
console.log("\nAfter first borrow:");
console.log("Debt balance =", ethers.formatEther(await debtToken.balanceOf(user1.address)));
console.log("Scaled debt balance =", ethers.formatEther(await debtToken.scaledBalanceOf(user1.address)));
console.log("Usage index_1 =", ethers.formatUnits(usageIndex1, 27));
expect(usageIndex1).to.be.closeTo(RAY, ethers.parseUnits("1", 18));
console.log("\nSkipping time to accrue interest...\n");
await ethers.provider.send("evm_increaseTime", [101 * 24 * 60 * 60]);
await ethers.provider.send("evm_mine", []);
await lendingPool.connect(user1).updateState();
const usageIndex2 = await lendingPool.getNormalizedDebt();
const intermediateDebt = await debtToken.balanceOf(user1.address);
console.log("After interest accrual:");
console.log("Debt balance =", ethers.formatEther(intermediateDebt));
console.log("Scaled debt balance =", ethers.formatEther(await debtToken.scaledBalanceOf(user1.address)));
console.log("Usage index_2 =", ethers.formatUnits(usageIndex2, 27));
expect(usageIndex2).to.be.closeTo(ethers.parseUnits("1.02", 27), ethers.parseUnits("0.0001", 27));
const secondBorrowAmount = ethers.parseEther("100");
await lendingPool.connect(user1).borrow(secondBorrowAmount);
const usageIndex3 = await lendingPool.getNormalizedDebt();
const finalDebt = await debtToken.balanceOf(user1.address);
console.log("\nAfter second borrow:");
console.log("Debt balance =", ethers.formatEther(finalDebt));
console.log("Scaled debt balance =", ethers.formatEther(await debtToken.scaledBalanceOf(user1.address)));
console.log("Usage index_3 =", ethers.formatUnits(usageIndex3, 27));
expect(usageIndex3).to.be.closeTo(usageIndex2, ethers.parseUnits("0.00000001", 27));
const expectedDebt = intermediateDebt + secondBorrowAmount;
expect(finalDebt).to.be.closeTo(expectedDebt, ethers.parseEther("0.1"));
});
Output:
LendingPool
After first borrow:
Debt balance = 500.0
Scaled debt balance = 500.0
Usage index_1 = 1.0
Skipping time to accrue interest...
After interest accrual:
Debt balance = 510.043899431944528669 ✔️
Scaled debt balance = 500.0 ✔️
Usage index_2 = 1.020087798863889057338012908 ✔️ <--- ~ 2%
After second borrow:
Debt balance = 620.289558695487088589 ❌ <--- should have been approximately 510 + 100 = 610
Scaled debt balance = 608.074676891859095817 ❌ <--- should have been approximately 500 + 100/1.02 = 598
Usage index_3 = 1.020087798863889057338012908 ✔️ <--- remains the same
1) demonstrates DebtToken mint bug
0 passing (8s)
1 failing
1) LendingPool
demonstrates DebtToken mint bug:
AssertionError: expected 620289558695487088589 to be close to 610043899431944528669 +/- 100000000000000000
Note that if you console.log the user.scaledDebtBalance value in LendingPool::borrow() after L358, it shows the figure in line with our expectations, which is approximately 598. Sadly that internal accounting isn't used as the source of truth everywhere and hence we find broken functionality at multiple places.
Mitigation
Make the following changes:
File: contracts/core/tokens/DebtToken.sol
136: function mint(
137: address user,
138: address onBehalfOf,
139: uint256 amount,
140: uint256 index
141: ) external override onlyReservePool returns (bool, uint256, uint256) {
142: if (user == address(0) || onBehalfOf == address(0)) revert InvalidAddress();
143: if (amount == 0) {
144: return (false, 0, totalSupply());
145: }
146:
147: uint256 amountScaled = amount.rayDiv(index);
148: if (amountScaled == 0) revert InvalidAmount();
149:
- 150: uint256 scaledBalance = balanceOf(onBehalfOf);
+ 150: uint256 scaledBalance = super.balanceOf(onBehalfOf);
151: bool isFirstMint = scaledBalance == 0;
152:
- 153: uint256 balanceIncrease = 0;
- 154: if (_userState[onBehalfOf].index != 0 && _userState[onBehalfOf].index < index) {
- 155: balanceIncrease = scaledBalance.rayMul(index) - scaledBalance.rayMul(_userState[onBehalfOf].index);
- 156: }
157:
158: _userState[onBehalfOf].index = index.toUint128();
159:
- 160: uint256 amountToMint = amount + balanceIncrease;
+ 160: uint256 amountToMint = amount;
161:
162: _mint(onBehalfOf, amountToMint.toUint128());
163:
164: emit Transfer(address(0), onBehalfOf, amountToMint);
- 165: emit Mint(user, onBehalfOf, amountToMint, balanceIncrease, index);
+ 165: emit Mint(user, onBehalfOf, amountToMint, 0, index);
166:
167: return (scaledBalance == 0, amountToMint, totalSupply());
168: }
