Summary
User cannot fully repay their debt if they borrow multiple times, due to incorrectly update to user's scaledDebtBalance.
Vulnerability Details
When a user borrows some reserve assets from LendingPool for the first time, assuming the borrowed amount is amount0 and reserve.usageIndex is usageIndex0, then the user's scaledDebtBalance is updated to amount0 / usageIndex0.
LendingPool.sol::borrow():
@> uint256 scaledAmount = amount.rayDiv(reserve.usageIndex);
@> (bool isFirstMint, uint256 amountMinted, uint256 newTotalSupply) = IDebtToken(reserve.reserveDebtTokenAddress).mint(msg.sender, msg.sender, amount, reserve.usageIndex);
IRToken(reserve.reserveRTokenAddress).transferAsset(msg.sender, amount);
@> user.scaledDebtBalance += scaledAmount;
Some DebtToken tokens are minted to the user, the minted amount is amount0 (balanceIncrease is 0).
DebtToken.sol::mint():
uint256 amountToMint = amount + balanceIncrease;
_mint(onBehalfOf, amountToMint.toUint128());
It is worth noting that DebtToken overrides ERC20's update() which is called by _mint(), and balances[user] is stored with amount0 / usageIndex0, same as the user's scaledDebtBalance.
DebtToken.sol::_update():
function _update(address from, address to, uint256 amount) internal virtual override {
if (from != address(0) && to != address(0)) {
revert TransfersNotAllowed();
}
@> uint256 scaledAmount = amount.rayDiv(ILendingPool(_reservePool).getNormalizedDebt());
super._update(from, to, scaledAmount);
emit Transfer(from, to, amount);
}
As time passes and interest accrues, reserve.usageIndex becomes higher than usageIndex0 and is updated to usageIndex1 (through ReserveLibrary.updateReserveInterests()).
When user repays their debt, user's DebtToken tokens are burned and user's scaledDebtBalance is reduced by the burned DebtToken token amount.
LendingPool.sol::_repay():
(uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) =
IDebtToken(reserve.reserveDebtTokenAddress).burn(onBehalfOf, amount, reserve.usageIndex);
IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled);
reserve.totalUsage = newTotalSupply;
@> user.scaledDebtBalance -= amountBurned;
In the burn() function of DebtToken, the returned burned amount is calculated as user debtToken balance / usage index (assuming the user repay fully).
uint256 userBalance = balanceOf(from);
...
if(amount > userBalance){
@> amount = userBalance;
}
@> uint256 amountScaled = amount.rayDiv(index);
if (amountScaled == 0) revert InvalidAmount();
_burn(from, amount.toUint128());
emit Burn(from, amountScaled, index);
return (amount, totalSupply(), amountScaled, balanceIncrease);
User's DebtToken balance increases over time as ERC20's balanceOf() is overriden.
DebtToken.sol::balanceOf()
function balanceOf(address account) public view override(ERC20, IERC20) returns (uint256) {
uint256 scaledBalance = super.balanceOf(account);
return scaledBalance.rayMul(ILendingPool(_reservePool).getNormalizedDebt());
}
Recall that balances[user] is amount0 / usageIndex0, then we have userBalance is (amount0 / usageIndex0) * usageIndex1, and amountScaled is userBalance / usageIndex1, the burned amount returned by burn() is amount0 / usageIndex0, the same as user's scaledDebtBalance, at last, user's scaledDebtBalance is set to 0.
Everything works fine if user borrows then repays, however, if user choose to borrow more reserve assets before repay, things would go wrong.
Assuming the borrowed amount is amount1, then:
The user's scaledDebtBalance is updated to amount0 / usageIndex0 + amount1 / usageIndex1;
The minted DebtToken token amount includes balanceIncrease which reflects the interests accured by the first borrow amount, therefore balances[user] is updated to amount0 / usageIndex0 + (amount1 + balanceIncrease) / usageIndex1.
If later when user then repays their debt (reserve.usageIndex is usageIndex2), then we have:
-
userBalance = (amount0 / usageIndex0 + (amount1 + balanceIncrease) / usageIndex1) * usageIndex2
-
amountScaled = userBalance / usageIndex2
= amount0 / usageIndex0 + (amount1 + balanceIncrease) / usageIndex1
Because user's scaledDebtBalance is amount0 / usageIndex0 + amount1 / usageIndex1, less than the returned burned amount (i.e. amountBurned), and the repay transaction will revert due to underflow error.
LendingPool.sol::_repay()
user.scaledDebtBalance -= amountBurned;
Impact
User won't be able to fully repay their debt, and their deposited NFTs are locked forever.
POC
Please run forge test --mt testAudit_UserCannotRepay.
pragma solidity ^0.8.19;
import {Test, console, stdError} from "forge-std/Test.sol";
import "../contracts/core/pools/LendingPool/LendingPool.sol";
import "../contracts/mocks/core/tokens/crvUSDToken.sol";
import "../contracts/core/tokens/RToken.sol";
import "../contracts/core/tokens/DebtToken.sol";
import "../contracts/core/tokens/RAACNFT.sol";
import "../contracts/core/primitives/RAACHousePrices.sol";
contract Audit is Test {
address owner = makeAddr("Owner");
address curveUSDVault;
LendingPool lendingPool;
RAACHousePrices raacHousePrices;
crvUSDToken crvUSD;
RToken rToken;
DebtToken debtToken;
RAACNFT raacNft;
function setUp() public {
raacHousePrices = new RAACHousePrices(owner);
crvUSD = new crvUSDToken(owner);
rToken = new RToken("RToken", "RToken", owner, address(crvUSD));
debtToken = new DebtToken("DebtToken", "DT", owner);
raacNft = new RAACNFT(address(crvUSD), address(raacHousePrices), owner);
lendingPool = new LendingPool(
address(crvUSD),
address(rToken),
address(debtToken),
address(raacNft),
address(raacHousePrices),
0.1e27
);
lendingPool.transferOwnership(owner);
vm.startPrank(owner);
raacHousePrices.setOracle(owner);
rToken.setReservePool(address(lendingPool));
debtToken.setReservePool(address(lendingPool));
vm.stopPrank();
vm.label(curveUSDVault, "Curve USD Vault");
vm.label(address(crvUSD), "crvUSD");
vm.label(address(rToken), "RToken");
vm.label(address(debtToken), "DebtToken");
vm.label(address(raacNft), "RAAC NFT");
vm.label(address(lendingPool), "LendingPool");
}
function testAudit_UserCannotRepay() public {
address bob = makeAddr("Bob");
crvUSD.mint(bob, 1000e18);
vm.startPrank(bob);
crvUSD.approve(address(lendingPool), 1000e18);
lendingPool.deposit(1000e18);
vm.stopPrank();
vm.prank(owner);
raacHousePrices.setHousePrice(1, 1000e18);
address alice = makeAddr("Alice");
crvUSD.mint(alice, 2000e18);
vm.startPrank(alice);
crvUSD.approve(address(raacNft), 1000e18);
raacNft.mint(1, 1000e18);
raacNft.approve(address(lendingPool), 1);
lendingPool.depositNFT(1);
lendingPool.borrow(100e18);
vm.stopPrank();
vm.warp(block.timestamp + 1 days);
vm.prank(alice);
lendingPool.borrow(100e18);
vm.warp(block.timestamp + 1 days);
vm.startPrank(alice);
crvUSD.approve(address(lendingPool), 1000e18);
vm.expectRevert(stdError.arithmeticError);
lendingPool.repay(type(uint256).max);
}
}
Tools Used
Manual Review
Recommendations
When borrows, user's scaledDebtBalance should be updated with minted DebtToken token amount including balanceIncrease.
// Update user's scaled debt balance
uint256 scaledAmount = amount.rayDiv(reserve.usageIndex);
// Mint DebtTokens to the user (scaled amount)
(bool isFirstMint, uint256 amountMinted, uint256 newTotalSupply) = IDebtToken(reserve.reserveDebtTokenAddress).mint(msg.sender, msg.sender, amount, reserve.usageIndex);
// Transfer borrowed amount to user
IRToken(reserve.reserveRTokenAddress).transferAsset(msg.sender, amount);
- user.scaledDebtBalance += scaledAmount;
+ user.scaledDebtBalance += amountMinted;
