Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Wrong returned values order of the DebtToken::burn() causes the protocol to lose funds

y0ng0p3

Summary

Wrong returned values order of the DebtToken::burn function causes the LendingPool::_repay function to transfer the amount to repay without interest accrued on borrowing, making the the protocol losing funds.

Vulnerability Details

The LendingPool::_repay function is used to repay or repay on behalf of another address in the lending pool. It invokes the DebtToken::burn function to burn the equivalent of debt tokens of the amount of assets the user want to repay. However, this function inverts the values of the amount of scaled tokens burned and the amount of underlying burned tokens.

File: contracts/core/tokens/DebtToken.sol#L170-L214
​
/**
* @notice Burns debt tokens from a user
* @param from The address from which tokens are burned
* @param amount The amount to burn (in underlying asset units)
* @param index The usage index at the time of burning
* @return A tuple containing: // @audit Here is the right order of the return values
* - uint256: The amount of scaled tokens burned
* - uint256: The new total supply after burning
* - uint256: The amount of underlying tokens burned
* - uint256: The balance increase due to interest
*/
function burn(
address from,
uint256 amount,
uint256 index
) external override onlyReservePool returns (uint256, uint256, uint256, uint256) {
if (from == address(0)) revert InvalidAddress();
if (amount == 0) {
return (0, totalSupply(), 0, 0);
}
​
uint256 userBalance = balanceOf(from);
​
uint256 balanceIncrease = 0;
if (_userState[from].index != 0 && _userState[from].index < index) {
uint256 borrowIndex = ILendingPool(_reservePool).getNormalizedDebt();
balanceIncrease = userBalance.rayMul(borrowIndex) - userBalance.rayMul(_userState[from].index);
amount = amount;
}
​
_userState[from].index = index.toUint128();
​
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); // @audit Should be `return (amountScaled, totalSupply(), amount, balanceIncrease)` instead
}

It makes the function transfer amount from the caller to reserve instead of the real amountScaled.

File: contracts/core/pools/LendingPool/LendingPool.sol#L398-L431
​
function _repay(uint256 amount, address onBehalfOf) internal {
if (amount == 0) revert InvalidAmount();
if (onBehalfOf == address(0)) revert AddressCannotBeZero();
​
UserData storage user = userData[onBehalfOf];
​
// Update reserve state before repayment
ReserveLibrary.updateReserveState(reserve, rateData);
​
// Calculate the user's debt (for the onBehalfOf address)
uint256 userDebt = IDebtToken(reserve.reserveDebtTokenAddress).balanceOf(onBehalfOf);
uint256 userScaledDebt = userDebt.rayDiv(reserve.usageIndex);
​
// If amount is greater than userDebt, cap it at userDebt
uint256 actualRepayAmount = amount > userScaledDebt ? userScaledDebt : amount;
​
uint256 scaledAmount = actualRepayAmount.rayDiv(reserve.usageIndex);
​
// Burn DebtTokens from the user whose debt is being repaid (onBehalfOf)
// is not actualRepayAmount because we want to allow paying extra dust and we will then cap there
(uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) = // @audit the values of `amountScaled` and `amount` have been inverted
IDebtToken(reserve.reserveDebtTokenAddress).burn(onBehalfOf, amount, reserve.usageIndex);
// Transfer reserve assets from the caller (msg.sender) to the reserve
IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled); // @audit It's `amount` that is in fact transfered
reserve.totalUsage = newTotalSupply;
user.scaledDebtBalance -= amountBurned;
// Update liquidity and interest rates
ReserveLibrary.updateInterestRatesAndLiquidity(reserve, rateData, amountScaled, 0);
​
emit Repay(msg.sender, onBehalfOf, actualRepayAmount);
}

amount is the amount the user want to repay and the amountScaled is the amount the user want to repay plus the interest accrued on borrowing.

Impact

This leads to the protocol losing the interests accrued on borrowing since the value of amountScaled transfered from the re-payer (the caller) to the reserve is in fact the value of amount.

Tools Used

Manual review.

Recommendations

Place amount and amountScaled in their right place when returning values in the DebtToken::burn function.

-- return (amount, totalSupply(), amountScaled, balanceIncrease);
++ return (amountScaled, totalSupply(), amount, balanceIncrease);
Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

DebtToken::burn calculates balanceIncrease (interest) but never applies it, allowing borrowers to repay loans without paying accrued interest

Interest IS applied through the balanceOf() mechanism. The separate balanceIncrease calculation is redundant/wrong. Users pay full debt including interest via userBalance capping.

DebtToken::burn returns items in the wrong order

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

DebtToken::burn calculates balanceIncrease (interest) but never applies it, allowing borrowers to repay loans without paying accrued interest

Interest IS applied through the balanceOf() mechanism. The separate balanceIncrease calculation is redundant/wrong. Users pay full debt including interest via userBalance capping.

DebtToken::burn returns items in the wrong order

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.