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

[M-3] Inconsistent scaling between `transfer` and `transferFrom` in `RToken` can lead to inflated balances

xkeywordx

Summary

The RToken contract overrides the standard ERC20 transfer and transferFrom functions in order to account for accrued interest by converting between the internal scaled balances and the underlying token amounts. However, the two functions use different methods for scaling.

Vulnerability Details

The RToken::transfer and balanceOf functions scale the amount using the current normalized income obtained from the LendingPool contract

function transfer(address recipient, uint256 amount) public override(ERC20, IERC20) returns (bool) {
//@audit -- scales based on `getNormalizedIncome` in `LendingPool`
uint256 scaledAmount = amount.rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
return super.transfer(recipient, scaledAmount);
}
​
function balanceOf(address account) public view override(ERC20, IERC20) returns (uint256) {
uint256 scaledBalance = super.balanceOf(account);
//@audit uses `getNormalizedIncome` as in `transfer()`
return scaledBalance.rayMul(ILendingPool(_reservePool).getNormalizedIncome());
}

whereas the RToken::transferFrom function scales the amount using the stored liquidity index in the RToken contract.

function transferFrom(address sender, address recipient, uint256 amount) public override(ERC20, IERC20) returns (bool) {
//@audit scales based on `_liquidityIndex` that represents cumulative interest
uint256 scaledAmount = amount.rayDiv(_liquidityIndex);
return super.transferFrom(sender, recipient, scaledAmount);
}

Since the normalized income will be updated continuously with interest accrual it can eventually diverge from the stored _liquidityIndex if there are no recent deposit/withdraw operations. These differences in scaling may lead to inconsistent token balances when transferring tokens.

If the protocol experiences a period of inactivity, then the “current” normalized income (which is updated continuously based on the block timestamp) may significantly differ from the stored _liquidityIndex. In such a case, transfers using transferFrom() instead of transfer() will inflate the amount of tokens received by the recipient.

This different logic in scaling breaks a core assumption about ERC20 tokens. Using one method over the other can inflate the perceived balance of the recipient.

Impact

  • Inconsistency in balances. Users transferring tokens via transfer versus transferFrom could receive different underlying amounts.

  • If the difference is big enough to allow for arbitrage, a user may transfer tokens between two wallets in order to artificially inflate their balances. This can dilute the token value if this user sells the extra tokens immediately after.

  • Breaks a core assumption about the ERC20 token. Users don't expect different results based on the transfer function that they use.

PoC

Add the following test inside the LendingPool.test.js file and create a new user inside the test file.

- let owner, user1, user2, user3;
+ let owner, user1, user2, user3, user4;
​
- [owner, user1, user2, user3] = await ethers.getSigners();
+ [owner, user1, user2, user3, user4] = await ethers.getSigners();
​
it.only("should demonstrate inconsistent scaling between transfer and transferFrom", async function () {
// Step 1: Have user1 deposit 1000 tokens, receiving RTokens.
const depositAmount = ethers.parseEther("1000");
await crvusd.connect(user1).approve(lendingPool.target, depositAmount);
await lendingPool.connect(user1).deposit(depositAmount);
// User1 borrows 100 tokens.
const borrowAmount = ethers.parseEther("100");
await lendingPool.connect(user1).borrow(borrowAmount);
// Step 2: Warp time so that interest accrues
const secondsInYear = 365 * 24 * 3600;
await ethers.provider.send("evm_increaseTime", [secondsInYear]);
await ethers.provider.send("evm_mine", []);
await lendingPool.connect(user1).updateState();
const normalizedIncome = await lendingPool.getNormalizedIncome();
console.log("Normalized Income:", ethers.formatUnits(normalizedIncome, 27));
// Step 3: user1 transfers 100 RTokens using transfer()
const transferUnderlying = ethers.parseEther("100");
const user4UnderlyingBefore = await rToken.balanceOf(user4.address);
console.log("User4 underlying before transfer():", ethers.formatEther(user4UnderlyingBefore));
// Now do the transfer:
await rToken.connect(user1).transfer(user4.address, transferUnderlying);
// Get the underlying equivalent for user4:
const user4UnderlyingAfter = await rToken.balanceOf(user4.address);
console.log("User4 underlying after transfer():", ethers.formatEther(user4UnderlyingAfter));
// Step 4: user1 transfers 100 RTokens using transferFrom():
const user3UnderlyingBefore = await rToken.balanceOf(user3.address);
console.log("User3 underlying before transferFrom():", ethers.formatEther(user3UnderlyingBefore));
// Now do the transfer using transferFrom:
await rToken.connect(user1).approve(user3.address, transferUnderlying);
await rToken.connect(user3).transferFrom(user1, user3.address, transferUnderlying);
// Get the underlying equivalent for user4:
const user3UnderlyingAfter = await rToken.balanceOf(user3.address);
console.log("User3 underlying after transferFrom():", ethers.formatEther(user3UnderlyingAfter));
expect(user3UnderlyingAfter).to.be.gt(user4UnderlyingAfter);
});

Test output

LendingPool
Borrow and Repay
Normalized Income: 1.000937500023341930030590004
User4 underlying before transfer(): 0.0
User4 underlying after transfer(): 99.906337805974891792
User3 underlying before transferFrom(): 0.0
User3 underlying after transferFrom(): 100.0
✔ should demonstrate inconsistent scaling between transfer and transferFrom (3324ms)
​
​
1 passing (15s)

As you can see the same 100 tokens transferred via transferFrom resulted in a bigger received amount.

Tools Used

Manual review

Recommendations

Use a unified scaling method. Standardize the scaling logic across all token transfer functions. For example, both transfer and transferFrom should use the current normalized income from the LendingPool or both should use the stored liquidity index (provided it is updated frequently enough) so that they produce identical conversions.

Updates

Lead Judging Commences

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

RToken::transfer uses getNormalizedIncome() while transferFrom uses _liquidityIndex, creating inconsistent transfer amounts depending on function used

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

RToken::transfer uses getNormalizedIncome() while transferFrom uses _liquidityIndex, creating inconsistent transfer amounts depending on function used

Support

FAQs

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