Summary
The RToken contract applies scaling calculations multiple times, leading to inaccurate token transfer amounts.
Vulnerability Details
The RToken contract overrides the original ERC20::transfer() and ERC20::transferFrom() functions, incorporating scaling adjustments in both methods:
* @dev Overrides the ERC20 transfer function to use scaled amounts
* @param recipient The recipient address
* @param amount The amount to transfer (in underlying asset units)
*/
function transfer(address recipient, uint256 amount) public override(ERC20, IERC20) returns (bool) {
@> uint256 scaledAmount = amount.rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
return super.transfer(recipient, scaledAmount);
}
* @dev Overrides the ERC20 transferFrom function to use scaled amounts
* @param sender The sender address
* @param recipient The recipient address
* @param amount The amount to transfer (in underlying asset units)
*/
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);
}
However, the _update() function also performs a scaling operation, leading to unintended double-scaling, which ultimately results in incorrect transfer amounts:
* @dev Internal function to handle token transfers, mints, and burns
* @param from The sender address
* @param to The recipient address
* @param amount The amount of tokens
*/
function _update(address from, address to, uint256 amount) internal override {
uint256 scaledAmount = amount.rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
super._update(from, to, scaledAmount);
}
Poc
Since _liquidityIndex is initially set to 1e27, the transferFrom() function does not apply any effective scaling.
To illustrate the discrepancy between transfer() and transferFrom(), add the following test case to test/unit/core/pools/LendingPool/LendingPool.test.js and execute it:
describe("transferFrom vs transfer", function () {
beforeEach(async function () {
const depositAmount = ethers.parseEther("1000");
await crvusd.connect(user2).approve(lendingPool.target, depositAmount);
await lendingPool.connect(user2).deposit(depositAmount);
const tokenId = 1;
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 lendingPool.connect(user1).updateState();
await ethers.provider.send("evm_increaseTime", [365 * 24 * 60 * 60]);
await ethers.provider.send("evm_mine", []);
await lendingPool.connect(user1).updateState();
});
it("transfer()", async function () {
await rToken.connect(user2).transfer(user3.address, ethers.parseEther("50"));
console.log("user3 RToken balance:",await rToken.balanceOf(user3.address));
});
it("transferFrom()", async function () {
await rToken.connect(user2).approve(user3.address, ethers.parseEther("50"));
await rToken.connect(user3).transferFrom(user2.address, user3.address, ethers.parseEther("50"));
console.log("user3 RToken balance:",await rToken.balanceOf(user3.address));
});
});
output:
LendingPool
transferFrom vs transfer
Promise { <pending> }
user3 RToken balance: 49965843659851598201n
✔ transfer() (464ms)
Promise { <pending> }
user3 RToken balance: 50000000000000000000n
✔ transferFrom() (733ms)
The results demonstrate that the transfer() function incorrectly scales the amount due to redundant calculations, while transferFrom() maintains accuracy.
Impact
The redundant scaling in the RToken contract causes incorrect transfer amounts, leading to inconsistencies in token balances and potential miscalculations in the lending system.
Tools Used
Manual Review
Recommendations
Since _update() already handles the necessary scaling, additional scaling in transfer() and transferFrom() is unnecessary. The most effective solution is to remove the redundant scaling logic. The corrected implementation should result in the following test output:
LendingPool
transferFrom vs transfer
Promise { <pending> }
user3 RToken balance: 50000000000000000000n
✔ transfer() (347ms)
Promise { <pending> }
user3 RToken balance: 50000000000000000000n
✔ transferFrom() (907ms)
