Summary
The RToken implementation suffers from a double-scaling issue during token transfers, where the transfer amount is scaled twice: once in the RToken::transfer function and again in the RToken::_update function. This results in the transferred tokens being devalued unnecessarily, causing incorrect token balances and potential financial losses for users.
Vulnerability Details
When a user deposits assets via LendingPool::deposit, they are minted rTokens. See below:
* @notice Allows a user to deposit reserve assets and receive RTokens
* @param amount The amount of reserve assets to deposit
*/
function deposit(uint256 amount) external nonReentrant whenNotPaused onlyValidAmount(amount) {
ReserveLibrary.updateReserveState(reserve, rateData);
uint256 mintedAmount = ReserveLibrary.deposit(reserve, rateData, amount, msg.sender);
_rebalanceLiquidity();
emit Deposit(msg.sender, amount, mintedAmount);
}
* @notice Handles deposit operation into the reserve.
* @dev Transfers the underlying asset from the depositor to the reserve, and mints RTokens to the depositor.
* This function assumes interactions with ERC20 before updating the reserve state (you send before we update how much you sent).
* A untrusted ERC20's modified mint function calling back into this library will cause incorrect reserve state updates.
* Implementing contracts need to ensure reentrancy guards are in place when interacting with this library.
* @param reserve The reserve data.
* @param rateData The reserve rate parameters.
* @param amount The amount to deposit.
* @param depositor The address of the depositor.
* @return amountMinted The amount of RTokens minted.
*/
function deposit(
ReserveData storage reserve,
ReserveRateData storage rateData,
uint256 amount,
address depositor
) internal returns (uint256 amountMinted) {
if (amount < 1) revert InvalidAmount();
updateReserveInterests(reserve, rateData);
IERC20(reserve.reserveAssetAddress).safeTransferFrom(
msg.sender,
reserve.reserveRTokenAddress,
amount
);
(
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;
}
The amount of rTokens minted to the user are normalized to allow for value accrual as the liquidity index increases. This is done in RToken::_update which overides the normal ERC20 _update function. See below:
* @dev Internal function to handle token transfers, mints, and burns
* @param from The sender address
* @param to The recipient address
* @param amount The amount of tokens
*/
function _update(
address from,
address to,
uint256 amount
) internal override {
uint256 scaledAmount = amount.rayDiv(
ILendingPool(_reservePool).getNormalizedIncome()
);
super._update(from, to, scaledAmount);
}
The issue occurs when a user attempts to transfer tokens to another user, the amount is being scaled twice before it is being sent which devalues the rtokens unneccesarily. It is scaled once in RToken:: _update and again in RToken::transfer See below:
* @dev Overrides the ERC20 transfer function to use scaled amounts
* @param recipient The recipient address
* @param amount The amount to transfer (in underlying asset units)
*/
function transfer(
address recipient,
uint256 amount
) public override(ERC20, IERC20) returns (bool) {
uint256 scaledAmount = amount.rayDiv(
ILendingPool(_reservePool).getNormalizedIncome()
);
return super.transfer(recipient, scaledAmount);
}
As a result, the transferred tokens aren't reflected in the receiver's balance.
Proof Of Code (POC)
The following test was run in LendingPool.test.js in the "Borrow and Repay" describe block
it("transfering rtokens devalues them via double scaling issue", async function () {
const reserve = await lendingPool.reserve();
console.log("reserve", reserve.lastUpdateTimestamp);
const depositAmount = ethers.parseEther("100");
await lendingPool.connect(user1).borrow(depositAmount);
await time.increase(5000 * 24 * 60 * 60);
await ethers.provider.send("evm_mine");
await lendingPool.connect(user1).deposit(depositAmount);
const reservedata = await lendingPool.getAllUserData(user1.address);
console.log(`liqindex`, reservedata.liquidityIndex);
const user1RTokenBalance = await rToken.scaledBalanceOf(user1.address);
console.log("user1RTokenBalance", user1RTokenBalance);
const pretransferuser2RTokenBalance = await rToken.scaledBalanceOf(
user2.address
);
console.log(
"pretransferuser2RTokenBalance",
pretransferuser2RTokenBalance
);
assert(user1RTokenBalance < depositAmount);
const transferAmount = ethers.parseEther("50");
await rToken.connect(user1).transfer(user2.address, transferAmount);
const user2RTokenBalance = await rToken.scaledBalanceOf(user2.address);
console.log("user2RTokenBalance", user2RTokenBalance);
const amountTransferred =
user2RTokenBalance - pretransferuser2RTokenBalance;
console.log("amountTransferred", amountTransferred);
function raymul(
uint256 val1,
uint256 val2
) external pure returns (uint256) {
return val1.rayMul(val2);
}
function raydiv(
uint256 val1,
uint256 val2
) external pure returns (uint256) {
return val1.rayDiv(val2);
}
then deploy the contract with the following lines:
const reservelibrary = await ethers.getContractFactory(
"ReserveLibraryMock"
);
reserveLibrary = await reservelibrary.deploy();
*/
const normalizedincome = await lendingPool.getNormalizedIncome();
console.log("normalizedincome", normalizedincome);
const singlescaledtamount = await reserveLibrary.raydiv(
transferAmount,
normalizedincome
);
const amount1 = await rToken.amount1();
console.log("amount1", amount1);
assert(amountTransferred < singlescaledtamount);
const amounttransferredunscaled = await reserveLibrary.raymul(
amountTransferred,
normalizedincome
);
assert(amounttransferredunscaled < transferAmount);
});
Impact
Financial Loss: Users lose value when transferring rTokens, as the transferred amount is devalued due to double-scaling.
Incorrect Balances: Token balances for both the sender and recipient are incorrect, leading to accounting discrepancies.
Tools Used
Manual Review, Hardhat
Recommendations
To fix this issue, the double-scaling should be eliminated by ensuring the transfer amount is scaled only once. This can be achieved by modifying the RToken::transfer function to avoid scaling the amount before passing it to super.transfer.
Updated RToken::transfer Function
Remove the scaling logic from the transfer function, as scaling is already handled in _update:
* @dev Overrides the ERC20 transfer function to use unscaled amounts
* @param recipient The recipient address
* @param amount The amount to transfer (in underlying asset units)
*/
function transfer(
address recipient,
uint256 amount
) public override(ERC20, IERC20) returns (bool) {
return super.transfer(recipient, amount);
}
The transfer function now passes the unscaled amount to super.transfer. The _update function will handle the scaling of the amount, ensuring it is scaled only once. This ensures that the transferred amount is correctly reflected in the recipient's balance without unnecessary devaluation.
