Summary
every borrower would have DebtToken, documentation stated that the DebtToken calculate the debt by using index-based system similar to Aave's VariableDebtToken. But the current implementation are also adding increased balance by calculating the delta of current index and user's last index, which is unnecessary.
Vulnerability Details
mint function takes amountas param in underlying asset units, and then the amount would be scaled by the override function _update.
now we take a look at the main mint function:
DebtToken.sol#L136-L168
function mint(
address user,
address onBehalfOf,
uint256 amount,
uint256 index
) external override onlyReservePool returns (bool, uint256, uint256) {
if (user == address(0) || onBehalfOf == address(0)) revert InvalidAddress();
if (amount == 0) {
return (false, 0, totalSupply());
}
uint256 amountScaled = amount.rayDiv(index);
if (amountScaled == 0) revert InvalidAmount();
@> uint256 scaledBalance = balanceOf(onBehalfOf);
bool isFirstMint = scaledBalance == 0;
uint256 balanceIncrease = 0;
if (_userState[onBehalfOf].index != 0 && _userState[onBehalfOf].index < index) {
@> balanceIncrease = scaledBalance.rayMul(index) - scaledBalance.rayMul(_userState[onBehalfOf].index);
}
_userState[onBehalfOf].index = index.toUint128();
@> uint256 amountToMint = amount + balanceIncrease;
_mint(onBehalfOf, amountToMint.toUint128());
emit Transfer(address(0), onBehalfOf, amountToMint);
emit Mint(user, onBehalfOf, amountToMint, balanceIncrease, index);
return (scaledBalance == 0, amountToMint, totalSupply());
}
the first issue is amountToMintis added bybalanceIncreasewhich is unnecessary addition because if this implementation are using index-based system like VariableDebtToken from AAVE, then it is enough to calculate the interest by multiplying the borrower balance by the current index later, no need to calculate manually here.
DebtToken.sol#L223-L226
function balanceOf(address account) public view override(ERC20, IERC20) returns (uint256) {
uint256 scaledBalance = super.balanceOf(account);
@> return scaledBalance.rayMul(ILendingPool(_reservePool).getNormalizedDebt());
}
second issue is the calculation in balanceIncrease is wrong, because scaledBalancederived from balanceOfis already multiplied by index, so in balanceIncrease block no need to multiply scaledBalance by index again. nonetheless this second issue not really important because the balanceIncreaseshould not be calculated in the first place though.
because with this issue in current implementation, the borrower balance would increase significantly every borrow function called.
to run PoC provided, use the detailed step below:
Run npm i --save-dev @nomicfoundation/hardhat-foundry in the terminal to install the hardhat-foundry plugin.
Add require("@nomicfoundation/hardhat-foundry"); to the top of the hardhat.config.cjs file.
Run npx hardhat init-foundry in the terminal.
rename ReserveLibraryMock.solto ReserveLibraryMock.sol.bakinside test/unit/librariesfolder so it does not throw error.
Create a file “Test.t.sol” in the “test/” directory and paste the provided PoC.
pragma solidity ^0.8.19;
import "forge-std/Test.sol";
import "../contracts/core/tokens/DebtToken.sol";
import "../contracts/mocks/core/tokens/ERC20Mock.sol";
import "../contracts/libraries/math/WadRayMath.sol";
contract MockLendingPool {
DebtToken public debtToken;
uint256 normalizedDebt;
constructor(address _debtToken) {
debtToken = DebtToken(_debtToken);
normalizedDebt = 1e27;
}
function setNormalizedDebt(uint256 _normalizedDebt) external {
normalizedDebt = _normalizedDebt;
}
function getNormalizedDebt() external view returns (uint256) {
return normalizedDebt;
}
function mockMint(address caller, address onBehalfOf, uint256 amount) external returns (bool, uint256, uint256) {
return debtToken.mint(caller, onBehalfOf, amount, normalizedDebt);
}
}
contract DebtTokenTest is Test {
DebtToken public debtToken;
MockLendingPool public lendingPool;
using WadRayMath for uint256;
address owner = makeAddr("owner");
address borrower = makeAddr("borrower");
function setUp() public {
vm.startPrank(owner);
debtToken = new DebtToken("DebtToken", "DTKN", owner);
lendingPool = new MockLendingPool(address(debtToken));
debtToken.setReservePool(address(lendingPool));
vm.stopPrank();
}
function test_debtTokenMintedAmountIsIncorrectMultipleBorrow() public {
lendingPool.setNormalizedDebt(1.1e27);
uint256 index = lendingPool.getNormalizedDebt();
uint256 mintAmount = 100 ether;
uint256 expectedMintAmountScaled = mintAmount.rayDiv(index);
lendingPool.mockMint(owner, borrower, mintAmount);
uint256 scaledDebtTokenBalanceFirstMint = debtToken.scaledBalanceOf(borrower);
console.log("scaled debtTokenBalance: %d", scaledDebtTokenBalanceFirstMint);
console.log("expectedMintAmountScaled: %d", expectedMintAmountScaled);
assertEq(scaledDebtTokenBalanceFirstMint, expectedMintAmountScaled);
lendingPool.setNormalizedDebt(1.2e27);
index = lendingPool.getNormalizedDebt();
mintAmount = 100 ether;
expectedMintAmountScaled = mintAmount.rayDiv(index);
lendingPool.mockMint(owner, borrower, mintAmount);
uint256 scaledDebtTokenBalanceSecondMint = debtToken.balanceOf(borrower);
uint256 debtTokenIncreaseScaled = scaledDebtTokenBalanceSecondMint - scaledDebtTokenBalanceFirstMint;
console.log("scaled debtTokenBalance after second borrow: %d", scaledDebtTokenBalanceSecondMint);
console.log("debtTokenIncreaseScaled: %d", debtTokenIncreaseScaled);
console.log("expectedMintAmountScaled: %d", expectedMintAmountScaled);
assertEq(debtTokenIncreaseScaled, expectedMintAmountScaled);
}
run the command forge t --mt test_debtTokenMinted -vvand the code would fail:
Ran 1 test for test/DebtToken.t.sol:DebtTokenTest
[FAIL: assertion failed: 129090909090909090910 != 83333333333333333333] test_debtTokenMintedAmountIsIncorrectMultipleBorrow() (gas: 135877)
Logs:
scaled debtTokenBalance: 90909090909090909091
expectedMintAmountScaled: 90909090909090909091
scaled debtTokenBalance after second borrow: 220000000000000000001
debtTokenIncreaseScaled: 129090909090909090910
expectedMintAmountScaled: 83333333333333333333
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 967.45µs (279.05µs CPU time)
Ran 1 test suite in 9.04ms (967.45µs CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/DebtToken.t.sol:DebtTokenTest
[FAIL: assertion failed: 129090909090909090910 != 83333333333333333333] test_debtTokenMintedAmountIsIncorrectMultipleBorrow() (gas: 135877)
we can see by the log above:
1) in first assert, where borrowing is done first time, borrowing 100 etherworth of asset would lead to minting ~90.9 etherof debtToken (scaled form), the correct amount with current debt index of 1.1. This is expected because when user borrow for the first time, the code block of balanceIncrease does not execute.
2) in second assert, where borrowing is done 2 times at different index, by borrowing another 100 ether worth of asset, the scaled debtTokenBalance increased by 129 ether. where the correct amount is 83.3 etherof scaled debt token. we can confirm this by multiplying 83.3 ether with 1.2 index, that would result in 100 etherthis is the exact amount of the borrowing amount done in this second assert.
subsequent borrowing would increased this value further because of the unnecessary balanceIncrease logic inside the mint function.
Impact
borrower debt significantly increase in everytime they call LendingPool::borrowresulting in potential liquidation because the borrower debt is growing at rapid pace in each borrow they do.
Tools Used
manual review
Recommendations
solve the current issue by properly use the correct variable, removing the balanceIncrease when minting:
@@ -147,22 +147,18 @@ contract DebtToken is ERC20, ERC20Permit, IDebtToken, Ownable {
uint256 amountScaled = amount.rayDiv(index);
if (amountScaled == 0) revert InvalidAmount();
uint256 scaledBalance = balanceOf(onBehalfOf);
bool isFirstMint = scaledBalance == 0;
- uint256 balanceIncrease = 0;
- if (_userState[onBehalfOf].index != 0 && _userState[onBehalfOf].index < index) {
- balanceIncrease = scaledBalance.rayMul(index) - scaledBalance.rayMul(_userState[onBehalfOf].index);
- }
-
_userState[onBehalfOf].index = index.toUint128();
- uint256 amountToMint = amount + balanceIncrease;
+ uint256 amountToMint = amount;
_mint(onBehalfOf, amountToMint.toUint128());
emit Transfer(address(0), onBehalfOf, amountToMint);
- emit Mint(user, onBehalfOf, amountToMint, balanceIncrease, index);
+ emit Mint(user, onBehalfOf, amountToMint, 0, index);
return (scaledBalance == 0, amountToMint, totalSupply());
