Vulnerability Details
The RToken::mint first projects Mint event with amountToMint before returning minted values. Then,RToken::mint returns amountToMint in place of amountScaled. This value is then returned to LendingPool::depositwhere mint amount is emitted.
RToken::mint:
@> emit Mint(caller, onBehalfOf, amountToMint, index);
...
@> return (isFirstMint, amountToMint, totalSupply(), amountScaled);
ReserveLibrary::deposit:
(bool isFirstMint, uint256 amountScaled, uint256 newTotalSupply, uint256 amountUnderlying) = IRToken(reserve.reserveRTokenAddress).mint(
address(this),
depositor,
amount,
reserve.liquidityIndex
);
@> amountMinted = amountScaled;
updateInterestRatesAndLiquidity(reserve, rateData, amount, 0);
@> emit Deposit(depositor, amount, amountMinted);
@> return amountMinted;
LendingPool::deposit:
@> uint256 mintedAmount = ReserveLibrary.deposit(reserve, rateData, amount, msg.sender);
_rebalanceLiquidity();
@> emit Deposit(msg.sender, amount, mintedAmount);
Issue 1:
The amountToMint returned & emitted is the amount that user deposited. This could be different from the actual amount minted since the amount that's minted is scaled; it is influenced by the reserve.liquidityIndex via _update function so the amounts emitted should contain balanceIncrease to align with Aave's AToken implementation which uses it for emit purpose.
The balanceIncrease calculation is missing in RToken::burn, but same issues are present there. It should use similar implementation used in Aave's AToken to align with it. It's provided in mitigation section,
The DebtToken faces similar issues which need adjustments.
Issue2:
The balanceIncrease calculations in RToken & DebtToken make use of output from balanceOf which returns actual balance, not the scaled balance. This value is then further unscaled when calculating balanceIncrease.
For clarification, in this report, scaled or normalized implies: value.rayDiv(index).
unscaled or unnormalized or actual value implies: value.rayMul(index).
For instance, here in RToken::mint
@> uint256 scaledBalance = balanceOf(onBehalfOf);
uint256 balanceIncrease = 0;
if (_userState[onBehalfOf].index != 0 && _userState[onBehalfOf].index < index) {
@> balanceIncrease = scaledBalance.rayMul(index) - scaledBalance.rayMul(_userState[onBehalfOf].index);
}
As per AToken implementation, balanceIncrease is calculated by using output from super.balanceOf(user) rather than from balanceOf that returns super.balanceOf(user).rayMul(index).
Recommendations
Make the following adjustments,
RToken::mint:
- uint256 scaledBalance = balanceOf(onBehalfOf); // @ this is already unscaled
+ uint256 scaledBalance = scaledBalanceOf(onBehalfOf);
- emit Mint(caller, onBehalfOf, amountToMint, index);
+ emit Mint(caller, onBehalfOf, amountToMint+balanceIncrease, index);
...
- return (isFirstMint, amountToMint, totalSupply(), amountScaled);
+ return (isFirstMint, amountToMint+balanceIncrease, totalSupply(), amountToMint);
RToken::burn : Here, balanceIncrease actually denotes deposit growth. So if protocol wants the burned amount, no need to emit Mint. The recommended mitigation aligns with Aave's AToken
- uint256 userBalance = balanceOf(user);
+ uint256 userBalance = scaledBalanceOf(user);
+ uint256 balanceIncrease = userBalance.rayMul(index) - userBalance.rayMul(_userState[user].index);
+ uint256 amountBurned = balanceIncrease > amount ? 0 : amount - balanceIncrease
+ if (balanceIncrease > amount) {
+ uint256 amountToMint = balanceIncrease - amount;
+ emit Mint(caller, onBehalfOf, amountToMint, index);
+ } else {
+ uint256 amountToBurn = amount - balanceIncrease;
- emit Burn(from, receiverOfUnderlying, amount, index);
+ emit Burn(from, receiverOfUnderlying, amountToBurn, index);
+ }
- return (amount, totalSupply(), amount);
+ return (amountBurned, totalSupply(), amount);
DebtToken::mint:
uint256 amountScaled = amount.rayDiv(index);
- uint256 scaledBalance = balanceOf(onBehalfOf); // @ this is already unscaled
+ uint256 scaledBalance = scaledBalanceOf(onBehalfOf);
- emit Transfer(address(0), onBehalfOf, amountToMint);
- emit Mint(user, onBehalfOf, amountToMint, balanceIncrease, index);
+ emit Transfer(address(0), onBehalfOf, amountScaled);
+ emit Mint(user, onBehalfOf, amountScaled, balanceIncrease, index);
- return (scaledBalance == 0, amountToMint, totalSupply());
+ return (scaledBalance == 0, amountScaled, totalSupply());
DebtToken::burn:
- uint256 scaledBalance = balanceOf(onBehalfOf); // @ this is already unscaled
+ uint256 scaledBalance = scaledBalanceOf(onBehalfOf);
...
...
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;
}
...
...
+ uint256 amountBurned = balanceIncrease > amount ? 0 : amount - balanceIncrease
+ if (balanceIncrease > amount) {
+ uint256 amountToMint = balanceIncrease - amount;
+ emit Mint(user, onBehalfOf, amountToMint, balanceIncrease, index);
+ } else {
+ uint256 amountToBurn = amount - balanceIncrease;
- emit Burn(from, amountScaled, index);
+ emit Burn(from, amountToBurn, index);
+ }
...
...
- return (amount, totalSupply(), amount);
+ return (amountBurned, totalSupply(), amount);
