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

Flaw in RToken transferFrom() function causes overminting of DETokens in StabilityPool and allows an attacker to drain the pool

BBnDD

Summary

The deposit() function in StabilityPool.sol incorrectly descales the deposit amount twice, leading to undercharging users for their deposits while overminting DETokens. The issue arises because the function calls safeTransferFrom(), which internally descales the amount by _liquidityIndex in transferFrom().

This results in significantly fewer RTokens being transferred from the user than intended, while minting DETokens as if the full amount was deposited.

Vulnerability Details

The amount parameter in StabilityPool#deposit is the RToken amount a user wishes to deposit. The function deposit() calls safeTransferFrom(user, StabilityPool, amount) as we can see below.

/**
* @notice Allows a user to deposit rToken and receive deToken.
* @param amount Amount of rToken to deposit.
*/
function deposit(uint256 amount) external nonReentrant whenNotPaused validAmount(amount) {
​
_update();
​
>>> rToken.safeTransferFrom(msg.sender, address(this), amount);
​
uint256 deCRVUSDAmount = calculateDeCRVUSDAmount(amount);
​
deToken.mint(msg.sender, deCRVUSDAmount);
​
​
userDeposits[msg.sender] += amount;
​
_mintRAACRewards();
​
​
emit Deposit(msg.sender, amount, deCRVUSDAmount);
​
}

safeTransferFrom() calls transferFrom function in RToken which is declared as:

/**
* @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);
​
}

We can see that the amount parameter is specified to be in underlying asset units. Then amount is descaled by _liquidityIndex and super.transferFrom is called.

Example:

  1. User wants to deposit 100 RTokens. Calls deposit(100).

  2. In the safeTransferFrom call the amount is divided by _liquidityIndex which is meant to represent the liquidity index of the reserve.

    Liquidity index = 4 (example value)
    100 / 4 = 25 RTokens

The actual amount that is taken from the user is 25 RTokens but 100 DETokens are minted to him which is incorrect. The minting of DETokens assumes the contract has received the amount provided to the function.

uint256 deCRVUSDAmount = calculateDeCRVUSDAmount(amount);
​
deToken.mint(msg.sender, deCRVUSDAmount);

The issue is that a user wants to withdraw the contract will send him 100 RTokens (because DETokens are converted 1:1 with RTokens). Essentially the user is stealing from other users even though it can be unintentional.

Impact

An attacker can drain all RToken deposits from the StabilityPool essentially stealing the lenders' deposits in the LendingPool.

Tools Used

manual review

Recommendations

Remove the logic for descaling the provided amount from the overridden transferFrom() function in RToken.sol . Make it behave like a regular ERC20 function.

Updates

Lead Judging Commences

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

RToken::transfer and transferFrom double-scale amounts by dividing in both external functions and _update, causing users to transfer significantly less than intended

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

RToken::transfer and transferFrom double-scale amounts by dividing in both external functions and _update, causing users to transfer significantly less than intended

Support

FAQs

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