Summary
The function RToken::transferFrom() uses stale liquidity index for calculation, which causes parties's balances to be updated incorrectly
Vulnerability Details
The state _liquidityIndex is set to WadRayMath.RAY in constructor and it can be updated by the function RToken::updateLiquidityIndex() which is meant to be called by LendingPool. Indeed, there is no function in the contract LendingPool that calls RToken::updateLiquidityIndex().
So, the function RToken::transferFrom() always use the stale value of _liquidityIndex = WadRayMath.RAY. As a result, the function _update() will update balances incorrectly
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);
}
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);
}
function _update(address from, address to, uint256 amount) internal override {
@> uint256 scaledAmount = amount.rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
super._update(from, to, scaledAmount);
}
PoC
Add test to file test/unit/core/pools/StabilityPool/StabilityPool.test.js
describe("Deposits", function () {
...
it.only("transferFrom() updates balance incorrectly", async function() {
const tokenId = 1;
const price = ethers.parseEther("100");
await raacHousePrices.setOracle(owner.address);
await raacHousePrices.setHousePrice(tokenId, price);
await crvusd.mint(user1.address, price)
await crvusd.connect(user1).approve(raacNFT.target, price)
await raacNFT.connect(user1).mint(tokenId, price);
await raacNFT.connect(user1).approve(lendingPool.target, tokenId);
await lendingPool.connect(user1).depositNFT(tokenId);
const borrowAmount = ethers.parseEther("50");
await lendingPool.connect(user1).borrow(borrowAmount);
await ethers.provider.send("evm_increaseTime", [86400]);
await ethers.provider.send("evm_mine");
await lendingPool.connect(user1).updateState();
console.log("\nbefore transfers")
let user1Balance = await rToken.balanceOf(user1.address);
let user3Balance = await rToken.balanceOf(user3.address);
console.log(`user1 ${user1Balance}\nuser3 ${user3Balance}`)
await rToken.connect(user2).transfer(user1.address, ethers.parseEther("10"))
await rToken.connect(user2).approve(user2.address, ethers.parseEther("10"))
await rToken.connect(user2).transferFrom(user2.address,user3.address, ethers.parseEther("10"))
user1Balance = await rToken.balanceOf(user1.address);
user3Balance = await rToken.balanceOf(user3.address);
console.log("\nafter transfers")
console.log(`user1 ${user1Balance}\nuser3 ${user3Balance}`)
})
Run the test and console shows:
StabilityPool
Core Functionality
Deposits
before transfers
user1 1000001171546476986016
user3 1000001171546476986016
after transfers
user1 1010001159831025941352
user3 1010001171546476986016
It means that the same amount is sent to user1 and user3 using transfer() and transferFrom() but it results different balances
Impact
Tools Used
Manual
Recommendations
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);
+ return super.transferFrom(sender, recipient, amount);
}
