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

Incorrect Decimal Scaling Formula in StabilityPool's `calculateRcrvUSDAmount` Function Leads to Incorrect amounts when withdrawing

tinnohofficial

Summary

The calculateRcrvUSDAmount function in StabilityPool contract uses incorrect decimal scaling when converting between DEToken and RToken amounts. This can lead to significant fund losses when users withdraw their deposits, either causing users to receive more RTokens than they originally deposited (protocol loss) or less RTokens than they should (user loss), depending on the decimal configurations of the tokens.

Vulnerability Details

When a user deposits RTokens to the StabilityPool, converting RToken → DEToken is done using the function calculateDeCRVUSDAmount which expressed in math terms as:

Substituting the values of scalingFactor and getExchangeRate():

For withdraw (DEToken → RToken), to maintain accuracy of formular, we need to make RAmt the subject;

Which simplifies to:

However when withdrawing, the rcrvUSDAmount calculated from calculateRcrvUSDAmount the current code uses introduces an incorrect scalingFactor of 10 ** (18 + rTokenDecimals - deTokenDecimals);

Looking at eqn and eqn they have different denominators, this shows that the used forumula in calculateRcrvUSDAmount function is WRONG.

This means when a user deposits RToken they will receive the correct amount of DEToken minted to them, but when they withdraw the value will be WRONGLY highy inflated.

PoC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {Test, console} from "forge-std/Test.sol";
import {StabilityPool} from "contracts/core/pools/StabilityPool/StabilityPool.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract MockERC20 is ERC20 {
uint8 private _decimals;
​
constructor(string memory name, uint8 decs) ERC20(name, name) {
_decimals = decs;
}
​
function decimals() public view override returns (uint8) {
return _decimals;
}
}
​
contract StabilityPoolTestPoC is Test {
address private alice = makeAddr("alice");
​
StabilityPool private stabilityPool;
StabilityPool private implementation;
ERC1967Proxy private proxy;
​
address private mockRToken = address(new MockERC20("RToken", 6));
address private mockDeToken = address(new MockERC20("DeToken", 18));
address private mockCrvUSDToken = address(new MockERC20("crvToken", 18));
address private raacToken = address(new MockERC20("RAACToken", 18));
address private mockRaacMinter = makeAddr("raac_minter");
address private mockLendingPool = makeAddr("lending_pool");
​
function setUp() public {
// Deploy StabilityPool implementation and proxy
implementation = new StabilityPool(address(this));
​
// I have just used mocks for this test since the specific token implementations is not relevant in this test
// except for the decimals part (of course)
bytes memory initData = abi.encodeWithSelector(
StabilityPool.initialize.selector,
mockRToken, // 6 decimals
mockDeToken, // 18 decimals
raacToken,
mockRaacMinter,
mockCrvUSDToken,
mockLendingPool
);
​
proxy = new ERC1967Proxy(address(implementation), initData);
stabilityPool = StabilityPool(address(proxy));
}
​
function testDepositAndWithdrawRToken() public view {
// assuming that alice has deposited her originalRAmount via the deposit function
uint256 originalRAmount = 5 * 1e6; // 5 RTokens
console.log("original RToken Amount: ", originalRAmount);
​
// the deposit function uses `calculateDeCRVUSDAmount` internally so for simplicity I will just use it here directly
// alice will get back the deAmount at a 1:1 (but remember decimals of each token is different)
uint256 deAmount = stabilityPool.calculateDeCRVUSDAmount(originalRAmount);
console.log("intermediate DeToken Amount: ", deAmount);
​
// assuming nothing happens then alice, just puts back the deAmount to withdraw her original R token,
// she should get get back an amount of rToken equal to originalRAmount right? WRONG!! this does not happen.
uint256 withdrawnRAmount = stabilityPool.calculateRcrvUSDAmount(deAmount);
console.log("New RToken Amount: ", withdrawnRAmount);
​
// alice withdraws significantly more than she deposited
assert(withdrawnRAmount > originalRAmount);
}
}

Impact

  • When deTokenDecimals > rTokenDecimals: Users withdraw significantly more RTokens than originally deposited, draining the pool

  • When deTokenDecimals < rTokenDecimals: Users receive significantly less RTokens than entitled, losing funds

  • Protocol's accounting system becomes fundamentally broken

Tools Used

  • Mathematical analysis

  • Foundry test framework

Recommendations

Update the calculateRcrvUSDAmount function to use the correct scaling formula:

function calculateRcrvUSDAmount(uint256 deCRVUSDAmount) public view returns (uint256) {
- uint256 scalingFactor = 10 ** (18 + rTokenDecimals - deTokenDecimals);
+ uint256 scalingFactor = 10 ** (18 + deTokenDecimals - rTokenDecimals);
return (deCRVUSDAmount * getExchangeRate()) / scalingFactor;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Incorrect scaling factor formula in StabilityPool::calculateRcrvUSDAmount function

Both tokens have 18 decimals. Info

Support

FAQs

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