Vulnerability Details
The current implementation of RToken::calculateDustAmount incorrectly calculates the totalRealBalance by using currentTotalSupply with rayMul(normalizedIncome). This is because the totalSupply() function returns the actual supply of tokens, not the scaled supply.
function totalSupply() public view override(ERC20, IERC20) returns (uint256) {
@> return super.totalSupply().rayMul(ILendingPool(_reservePool).getNormalizedIncome());
}
function calculateDustAmount() public view returns (uint256) {
uint256 contractBalance = IERC20(_assetAddress).balanceOf(address(this)).rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
@> uint256 currentTotalSupply = totalSupply();
@> uint256 totalRealBalance = currentTotalSupply.rayMul(ILendingPool(_reservePool).getNormalizedIncome());
return contractBalance <= totalRealBalance ? 0 : contractBalance - totalRealBalance;
}
So calculateDustAmount attempts to unscale the actual supply which has already been unscaled resulting in an overstated totalRealBalance. This skews the calculation (contractBalance - totalRealBalance) intended for transfer to the owner, leaving behind funds in the contract.
Although funds can be rescued using DebtToken::rescueToken by the reservePool, a patch is necessary for calculateDustAmount to serve its intended purpose.
Tools Used
Manual Review + Hardhat Testing
Proof-Of-Code
Place the following test in LendingPool.test.js,
it("test dust amounts not drained completely", async() => {
const depositAmount = ethers.parseEther("20");
const depositAmount2 = ethers.parseEther("30");
const depositAmount3 = ethers.parseEther("10");
await lendingPool.connect(user2).borrow(depositAmount2);
await debtToken.balanceOf(user2.address);
await lendingPool.connect(user3).borrow(depositAmount3);
await debtToken.balanceOf(user3.address);
await lendingPool.connect(user1).borrow(depositAmount);
await ethers.provider.send("evm_increaseTime", [5 * 24 * 60 * 60]);
await ethers.provider.send("evm_mine", []);
await lendingPool.updateState();
await crvusd.connect(user2).transfer(rToken.target, ethers.parseEther("100"));
const contractBalanceBeforeTransfer = await crvusd.balanceOf(rToken.target);
const totalRealBalanceNow = await rToken.totalSupply();
const expectedDustTransferred = contractBalanceBeforeTransfer - totalRealBalanceNow;
await lendingPool.connect(owner).transferAccruedDust(user1.address, contractBalanceBeforeTransfer);
const balanceNow = await crvusd.balanceOf(rToken.target);
const actualDustTransferred = contractBalanceBeforeTransfer - balanceNow;
expect(expectedDustTransferred).to.gt(actualDustTransferred);
})
Recommendations
Consider one of the following mitigations,
First Mitigation:
function calculateDustAmount() public view returns (uint256) {
// Calculate the actual balance of the underlying asset held by this contract
uint256 contractBalance = IERC20(_assetAddress).balanceOf(address(this)).rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
// Calculate the total real obligations to the token holders
uint256 currentTotalSupply = totalSupply();
// Calculate the total real balance equivalent to the total supply
- uint256 totalRealBalance = currentTotalSupply.rayMul(ILendingPool(_reservePool).getNormalizedIncome());
- return contractBalance <= totalRealBalance ? 0 : contractBalance - totalRealBalance;
+ return contractBalance <= totalRealBalance ? 0 : contractBalance - currentTotalSupply;
}
Second Mitigation:
function calculateDustAmount() public view returns (uint256) {
// Calculate the actual balance of the underlying asset held by this contract
uint256 contractBalance = IERC20(_assetAddress).balanceOf(address(this)).rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
// Calculate the total real obligations to the token holders
- uint256 currentTotalSupply = totalSupply();
+ uint256 currentTotalSupply = scaledTotalSupply();
// Calculate the total real balance equivalent to the total supply
uint256 totalRealBalance = currentTotalSupply.rayMul(ILendingPool(_reservePool).getNormalizedIncome());
return contractBalance <= totalRealBalance ? 0 : contractBalance - totalRealBalance;
}