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

rToken transfer and transferFrom functions actually transfer different amounts

CODESPECT

Summary

rToken is an ERC20 with overridden transfer functions. Using different functions will result in different amounts getting transferred.

Vulnerability Details

The rToken contract overrides the transfer and transferFrom functions to use scaled amounts of tokens. However, they are different between them resulting in different amounts getting transferred:

rToken.sol
​
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);
}

As we can see, at the transfer function the amount gets divided by the real time liquidityIndex of the LendingPool which is returned by the getNormalizedIncome() function:

LendingPool.sol
​
function getNormalizedIncome() external view returns (uint256) {
return reserve.liquidityIndex;
}

However, transferFrom uses an internal _liquidityIndex variable which is set as 1 RAY during contract construction and it is never updated again anywhere:

rToken.sol
​
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;
_assetAddress = assetAddress;
}
​
function updateLiquidityIndex(uint256 newLiquidityIndex) external override onlyReservePool {
if (newLiquidityIndex < _liquidityIndex) revert InvalidAmount();
_liquidityIndex = newLiquidityIndex;
emit LiquidityIndexUpdated(newLiquidityIndex);
}

There is a function to update this variable, only callable by the LendingPool address, but it is never called throughout the protocol. This means that _liquidityIndex will forever be equal to 1 RAY.

Impact

The transfer and transferFrom actually transfer different amount of tokens for the same amount input.

POC

To create a realistic scenario where liquidityIndex has increased and it's not equal to 1 RAY, paste the following function in the LendingPool.sol contract which sets the liquiditIndex = 1.1 RAY:

LendingPool.sol
​
function setNormalizedIncome() external {
reserve.liquidityIndex = 1.1e27;
}

Next, paste the following code in the test/unit/core/pools/LendingPool/LendingPool.test.js file under the describe("Deposit and Withdraw", function () { section:

it.only("should have different transferred amounts for different transfer functions", async function () {
const depositAmount = ethers.parseEther("100");
​
await lendingPool.connect(user1).deposit(depositAmount);
​
await ethers.provider.send("evm_mine", []);
let rTokenBalance = await rToken.balanceOf(user1.address);
expect(rTokenBalance).to.equal(depositAmount);
​
await lendingPool.setNormalizedIncome();
​
const transferAmount = ethers.parseEther("50");
​
await rToken.connect(user1).transfer(user2, transferAmount);
let rTokenBalance1 = await rToken.balanceOf(user1.address);
​
const transferredAmount1 = rTokenBalance - rTokenBalance1;
​
console.log("Transferred amount 1: ", transferredAmount1);
​
await rToken.connect(user1).approve(user1, transferAmount);
await rToken.connect(user1).transferFrom(user1, user2, transferAmount);
let rTokenBalance2 = await rToken.balanceOf(user1.address);
​
const transferredAmount2 = rTokenBalance1 - rTokenBalance2;
​
console.log("Transferred amount 2: ", transferredAmount2);
});

Tools Used

Manual review

Recommendations

Use the real time liquidityIndex at the transferFrom function like you use it in the transfer function:

function transferFrom(address sender, address recipient, uint256 amount)
public
override(ERC20, IERC20)
returns (bool)
{
-- uint256 scaledAmount = amount.rayDiv(_liquidityIndex);
++ uint256 scaledAmount = 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!