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

Discrepancy in `liquidityIndex` used in `transferFrom` leads to wrong amount being transferred: RToken.sol

fresh

Summary

In RToken.sol-> the liquidityIndexis used throughout the contract for the purpose of scaling the amount of RTokensthat either the protocol or user has in their balance.

The correct and most accurate and up-to-date liquidityIndexis used everywhere accept for in the function transferFrom.

Vulnerability Details

For all functions and actions that use the liquidityIndexfor scaling purposes, the most up-to-date and accurate value is used as the scaling factor, accept for transferFrom.

It is important to note that liquidityIndexis a value that is updated and stored within LendingPool(which is represented as _reservePoolin the RTokencontract.

The contract uses the most up-to-date index by these 2 methods:

  • Accepts the most up-to-date and accurate liquidityIndexvalue as an argument within a function. The argument is populated from the LendingPool, when calling these functions in RToken: as such:

// onlyReservePool ensures this function is only called by LendingPool
// `index` is the liquidityIndex -> which is populated by LendingPool, which always is the accurate index
function mint(
address caller,
address onBehalfOf,
uint256 amountToMint,
-> uint256 index
) external override onlyReservePool
​
​
// onlyReservePool ensures this function is only called by LendingPool
// `index` is the liquidityIndex -> which is populated by LendingPool, which always is the accurate index
function burn(
address from,
address receiverOfUnderlying,
uint256 amount,
-> uint256 index
) external override onlyReservePool
​

  • Calls the LendingPooldirectly and gets the return value of getNormalizedIncome-> which returns the accurate liquidityIndex

function balanceOf(address account) public view override(ERC20, IERC20) returns (uint256) {
uint256 scaledBalance = super.balanceOf(account);
-> return scaledBalance.rayMul(ILendingPool(_reservePool).getNormalizedIncome());
​
function totalSupply() public view override(ERC20, IERC20) returns (uint256) {
-> return super.totalSupply().rayMul(ILendingPool(_reservePool).getNormalizedIncome());
​
function transfer(address recipient, uint256 amount) public override(ERC20, IERC20) returns (bool) {
-> uint256 scaledAmount = amount.rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
return super.transfer(recipient, scaledAmount);

BUT the transferFromfunction uses _liquidityIndexfor the scaling factor. This variable is defined and stored within the RTokencontract itself.

  • It is originally set during the constructorof RToken:

uint256 private _liquidityIndex;
​
constructor(
string memory name,
string memory symbol,
address initialOwner,
address assetAddress
) ERC20(name, symbol) ERC20Permit(name) Ownable(initialOwner) {
if (initialOwner == address(0) || assetAddress == address(0)) revert InvalidAddress();
-> _liquidityIndex = WadRayMath.RAY;

  • There is a function, updateLiquidityIndex-> that is supposed to update the local liquidityIndex-> and is only callable by the LendingPool/reservePool. The reservePool is the LendingPool contract.

    /**
    * @notice Updates the liquidity index
    * @param newLiquidityIndex The new liquidity index
    */
    function updateLiquidityIndex(uint256 newLiquidityIndex) external override onlyReservePool {
    if (newLiquidityIndex < _liquidityIndex) revert InvalidAmount();
    _liquidityIndex = newLiquidityIndex;
    emit LiquidityIndexUpdated(newLiquidityIndex);

  • The problem is that this function is never called within the LendingPool. There is no point in the LendingPoolor the contracts inherited by the LendingPoolthat call this function. Therefore, the local _liquidityIndexis never updated and is continuously the value it was set as in the cosntructor.

When transferFromis called, it will use a different liquidityIndexthan the rest of the contract uses.

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);

Impact

To compare the discrepancy lets look at the difference between transferand transferFrom-> which both attempt to scale the value by the liquidityIndex:

** This uses the accurate liquidityIndex , getting it directly from the LendingPool
** at the time of calling this function, which means it is the up-to-date 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);
}
​
​
** This uses the local variable _liquidityIndex
** Which is never updated to the actual index value from the LendingPool
​
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);

The wrong amount will be transferred when transferFromis called.

I have this as a LOW becasue the protocol does not lose any funds, but the discrepency in the value can cause a user to lose funds and have the wrong amount transferred out. Also, it can have the reverse effect.

Tools Used

Manual Review

Recommendations

Follow the methods of using the liquidityIndexas the rest of the contract does. In this scenario, since it is a public variable and not accepting input paramater of liquidityIndex-> Use the logic in transfer

get the liquidityIndexby calling the lending pool directly - amount.rayDiv(ILendingPool(_reservePool).getNormalizedIncome());

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

Lead Judging Commences

inallhonesty Lead Judge 7 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 7 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.

Give us feedback!