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

RToken::transfer & transferFrom do not use the same index. Value transfer will not be the same depending on the method called

foufrix

Summary

In RToken, user can either transfer money using transfer() or transferFrom().

The issue is that they both compute the value that should be transferred using different index values.

Vulnerability Details

transfer() and transferFrom() do not use the same index as a reference for transferring value :

function transfer(address recipient, uint256 amount) public override(ERC20, IERC20) returns (bool) {
uint256 scaledAmount = amount.rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
return super.transfer(recipient, scaledAmount);
}
​
function transferFrom(address sender, address recipient, uint256 amount) public override(ERC20, IERC20) returns (bool) {
uint256 scaledAmount = amount.rayDiv(_liquidityIndex);
return super.transferFrom(sender, recipient, scaledAmount);
}

transfer() uses the index computed by the LendingPool while transferFrom() uses RToken::_liquidityIndex.

The major problem here is that those values will be different, and also that RToken::_liquidityIndex will never be updated as updateLiquidityIndex() onlyReservePool is never called in the LendingPool/ReservePool.

Let's take userA who wants to transfer 100 tokens.
Using a modern wallet, he will see the value of 100 in his wallet that calls balanceOf().
Let's take _liquidityIndex = 1
And ILendingPool(_reservePool).getNormalizedIncome() = 1,1
For an index of 1,1, this will end has a nonscaled value of 90,9.

Two scenarios:

  • userA calls transfer() with a value of 50

  • userA will transfer 45,45 non scaled token

  • userA calls transferFrom() with a value of 50

  • userA will transfer 50 nonscaled tokens, meaning 55 scaled tokens

Impact

Depending on the function they call, users will transfer different amount of tokens. The more the LendingPool index grows, the bigger the difference will become.
Also all contracts that interact with RToken will have different outputs if they use transfer() or transferFrom(). This can lead to improper accounting into external contracts.
For example, deposit() in StabilityPool may fail and user will deposit more than they thought they would.

Tools Used

Manual

Recommendations

Fix transferFrom() so that it uses the proper index value :

function transferFrom(address sender, address recipient, uint256 amount) public override(ERC20, IERC20) returns (bool) {
uint256 scaledAmount = amount.rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
return super.transferFrom(sender, recipient, scaledAmount);
}
Updates

Lead Judging Commences

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

RToken::updateLiquidityIndex() has onlyReservePool modifier but LendingPool never calls it, causing transferFrom() to use stale liquidity index values

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

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

RToken::updateLiquidityIndex() has onlyReservePool modifier but LendingPool never calls it, causing transferFrom() to use stale liquidity index values

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.