Core Contracts

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

Some RToken Dust would end locked inside contract because of wrong calculation inside calculateDustAmount

Summary

calculateDustAmount returns amount of tokens, which is not owned by the users. So it could be transfered out, but because usdCSV amount is multiplied twice by normalizedIncome. Protocol assumes users have more tokens that they actually do.

Vulnerability Details

Contract is calculating contractBalance which is equal to amount of CRVusd tokens owned by the contract. And this is calculated correctly, because CRVusd amount in first multiplied by normalizedIncome inside balanceOf, so here it is divided by the same value, to get back original CRVusd amount.

But the problem is in calculating totalRealBalance, it calls totalSupply which is also multiplies CRVusd amount by normalizedIncome, so it includes the income. And later in line 17 of provided code snippet it multplies that value again by normalizedIncome, which is wrong.

Probably protocol also wanted to use division there to get back original amount of CRVusd token so it can compare this value to contractBalance, but wrote multiply by mistake.

// RToken.sol 203
function totalSupply() public view override(ERC20, IERC20) returns (uint256) {
return super.totalSupply().rayMul(ILendingPool(_reservePool).getNormalizedIncome());
}
// RToken.sol 317
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());
// All balance, that is not tied to rToken are dust (can be donated or is the rest of exponential vs linear)
return contractBalance <= totalRealBalance ? 0 : contractBalance - totalRealBalance;
}

This function is used by transferAccruedDust, so it actually calculates how many tokens can be transferred from the contract. Because of cap in line 358 (line 10 in code snippet below), ReservePool wouldn't be able to transfer more tokens, than calculateDustAmount allows to

// RToken.sol 351
function transferAccruedDust(address recipient, uint256 amount) external onlyReservePool {
if (recipient == address(0)) revert InvalidAddress();
uint256 poolDustBalance = calculateDustAmount();
if(poolDustBalance == 0) revert NoDust();
// Cap the transfer amount to the actual dust balance
uint256 transferAmount = (amount < poolDustBalance) ? amount : poolDustBalance;
// Transfer the amount to the recipient
IERC20(_assetAddress).safeTransfer(recipient, transferAmount);
emit DustTransferred(recipient, transferAmount);
}

Impact

Because normalizedIncome will always be greater than or equal to 1. totalRealBalance will be bigger than it should be, so some tokens would end up locked inside the contract.

Tools Used

Manual Review

Recommendations

One way of soliving it is to replace rayMul with rayDiv for totalRealBalance calculation

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

RToken::calculateDustAmount incorrectly applies liquidity index, severely under-reporting dust amounts and permanently trapping crvUSD in contract

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!