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

Multiple issues from double scaling in RToken transfer amount calculation

glightspeed2

Summary

Due to double scaling in RToken.transferand RToken.transferFrom, DEToken holders will suffer from fund loss on withdrawal. Morever, RToken can be double-spent.

Vulnerability Details

Root Cause Analysis

When RTokens are transferred, RToken calculates and transfers scaledAmount of underlying asset.

In transfer, scaledAmount = amount / lendingPool.liquidityIndex:

function transfer(address recipient, uint256 amount) public override(ERC20, IERC20) returns (bool) {
uint256 scaledAmount = amount.rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
return super.transfer(recipient, scaledAmount);
}

In transferFrom, scaledAmount = amount / RToken.liquidityIndex:

function transferFrom(address sender, address recipient, uint256 amount) public override(ERC20, IERC20) returns (bool) {
uint256 scaledAmount = amount.rayDiv(_liquidityIndex); // @audit different liquidity index
return super.transferFrom(sender, recipient, scaledAmount);
}

As we can see, there is inconsistency between scaledAmount calculations of transfer and transferFrom. (check NOTE at the bottom of this report)

Morover, double scaling happens in RToken._update:

function _update(address from, address to, uint256 amount) internal override {
// Scale amount by normalized income for all operations (mint, burn, transfer)
uint256 scaledAmount = amount.rayDiv(ILendingPool(_reservePool).getNormalizedIncome());
super._update(from, to, scaledAmount);
}

Let's check how this double scaling affects the token behavior. Consider the following scenario:

  • Current liquidityIndex is 1

  • Alice deposits 10 USD to LendingPool and receives 10 RToken.

  • liquidityIndex is increased to 2

  • Alice's RToken balance increased to 20

  • Alice transferrs 10 RToken to Bob

    • Transferred underlying balance is 10 / liquidityIndex / liquidityIndex = 2.5

    • Alice's underlying balance is 7.5. and RToken balance is 15

    • Bob's underlying balance is 2.5 and RToken balance is 5

  • Bob transferrs all his balance back to Alice. He wants to repay 10 RToken but he cannot because his balance is only 5 due to the bug

    • Transferred underlying balance is 5 / liquidityIndex / liquidityIndex = 1.25

    • Alice's underlying balance is 8.75 and RToken balance is 17.5

    • Bob's underlying balance is 1.25 and RToken balance is 2.5

  • Alice withdraws all his RToken from LendingPool. LendingPool returns only 8.75 USD even though RToken claimed amount is 17.5 due to this check

In the above thought experiment, we observed the following two issues:

  • Double spend problem: Bob transferrs all balance 5 back to Alice but his RToken balance is still non-zero value: 2.5

  • User fund loss: Alice lost 1.25 USD after interacting with Bob

Of course, a normal user won't transfer RToken back and forth with other users. But in this protocol, exact same scenario must happen: it's when users deposit RToken to and withdraw RToken from StabilityPool. i.e. StabilityPool is Bob

POC

Scenario - DEHolder loses fund

  • Depositor deposits 10_000 crvUSD to LendingPool and receives some RToken

  • Depositor deposits those RToken to StabilityPool and receives some DEToken

  • Borrower mints an NFT worth of 20_000 crvUSD

  • Borrower deposits the NFT into LendingPool

  • 100 day passes

  • Borrower fully repays their debt

  • Depositor withdraws all DEToken from StabilityPool and receives some RToken

  • Depositor withdraws all RToken from LendingPool and receives crvUSD...

  • Depositor is left with around 9_012.34567 crvUSD. He loses nearly 1k USD due to the mentioned vulnerability

Scenario - RToken holder can double spend

  • Depositor deposits 10_000 crvUSD to LendingPool and receives some RToken

  • Depositor deposits those RToken to StabilityPool and receives some DEToken

  • Borrower mints an NFT worth of 20_000 crvUSD

  • Borrower deposits the NFT into LendingPool

  • 100 day passes

  • Borrower fully repays their debt

  • Depositor withdraws all DEToken from StabilityPool and receives some RToken

  • Depositor transferrs all RToken to another user

  • Depositor's remaining balance is greater than zero

How to run the PoC

First, follow the steps in foundry-hardhat integration tutorial

Then, create a file test/poc.t.sol and put the following content, and run forge test poc.t.sol -vvv

pragma solidity ^0.8.19;
​
import "../lib/forge-std/src/Test.sol";
import {RToken} from "../contracts/core/tokens/RToken.sol";
import {DebtToken} from "../contracts/core/tokens/DebtToken.sol";
import {DEToken} from "../contracts/core/tokens/DEToken.sol";
import {RAACToken} from "../contracts/core/tokens/RAACToken.sol";
import {RAACNFT} from "../contracts/core/tokens/RAACNFT.sol";
import {LendingPool} from "../contracts/core/pools/LendingPool/LendingPool.sol";
import {ILendingPool} from "../contracts/interfaces/core/pools/LendingPool/ILendingPool.sol";
import {StabilityPool} from "../contracts/core/pools/StabilityPool/StabilityPool.sol";
import {RAACMinter} from "../contracts/core/minters/RAACMinter/RAACMinter.sol";
import {crvUSDToken} from "../contracts/mocks/core/tokens/crvUSDToken.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​
contract RAACHousePricesMock {
mapping(uint256 => uint256) public prices;
​
function getLatestPrice(uint256 tokenId) external view returns (uint256, uint256) {
return (prices[tokenId], block.timestamp);
}
​
function setTokenPrice(uint256 tokenId, uint256 price) external {
prices[tokenId] = price;
}
​
function tokenToHousePrice(uint256 tokenId) external view returns (uint256) {
return prices[tokenId];
}
}
​
contract RAACTest is Test {
RToken rtoken;
DebtToken debtToken;
DEToken deToken;
RAACToken raacToken;
RAACNFT raacNft;
RAACMinter raacMinter;
LendingPool lendingPool;
StabilityPool stabilityPool;
RAACHousePricesMock housePrice;
crvUSDToken asset;
​
address depositor = makeAddr("depositor");
address borrower = makeAddr("borrower");
​
uint256 tokenId = 1;
uint256 userAssetAmount = 10_000e18;
uint256 nftPrice = 20_000e18;
​
uint256 initialBurnTaxRate = 50;
uint256 initialSwapTaxRate = 100;
uint256 initialPrimeRate = 0.1e27;
​
function setUp() external {
vm.warp(1e9); // warp time stamp to avoid underflow in RAACMinter constructor
// setup test bed
housePrice = new RAACHousePricesMock();
asset = new crvUSDToken(address(this));
debtToken = new DebtToken("DebtToken", "DTK", address(this));
rtoken = new RToken("RToken", "RTK", address(this), address(asset));
deToken = new DEToken("DEToken", "DE", address(this), address(rtoken));
raacToken = new RAACToken(address(this), initialSwapTaxRate, initialBurnTaxRate);
raacToken.setMinter(address(this));
raacNft = new RAACNFT(address(asset), address(housePrice), address(this));
lendingPool = new LendingPool(
address(asset), address(rtoken), address(debtToken), address(raacNft), address(housePrice), initialPrimeRate
);
rtoken.setReservePool(address(lendingPool));
debtToken.setReservePool(address(lendingPool));
stabilityPool = new StabilityPool(address(this));
stabilityPool.initialize(
address(rtoken), address(deToken), address(raacToken), address(this), address(asset), address(lendingPool)
);
raacMinter = new RAACMinter(address(raacToken), address(stabilityPool), address(lendingPool), address(this));
stabilityPool.setRAACMinter(address(raacMinter));
deToken.setStabilityPool(address(stabilityPool));
​
vm.label(address(asset), "crvUSD");
​
// depositor starts with 10_000 USD
deal(address(asset), depositor, userAssetAmount);
assertEq(asset.balanceOf(depositor), userAssetAmount);
​
// depositor deposits crvUSD to lending pool
vm.startPrank(depositor);
asset.approve(address(lendingPool), userAssetAmount);
lendingPool.deposit(userAssetAmount);
uint256 rtokenBalance = rtoken.balanceOf(depositor);
// depositor deposits received rToken to stability pool
rtoken.approve(address(stabilityPool), rtokenBalance);
stabilityPool.deposit(rtokenBalance);
vm.stopPrank();
​
// borrower mints an NFT worth 20_000 USD
housePrice.setTokenPrice(tokenId, nftPrice);
deal(address(asset), borrower, nftPrice);
vm.startPrank(borrower);
asset.approve(address(raacNft), nftPrice);
raacNft.mint(tokenId, nftPrice);
raacNft.approve(address(lendingPool), tokenId);
// borrower deposits minted NFT to lending pool
lendingPool.depositNFT(tokenId);
// borrower borrows 10_000 USD from pool
lendingPool.borrow(userAssetAmount);
vm.stopPrank();
​
// 100 day passes
skip(8640000);
lendingPool.updateState();
​
// borrower repays all their debt
vm.startPrank(borrower);
uint256 debt = debtToken.balanceOf(borrower);
deal(address(asset), borrower, debt);
asset.approve(address(lendingPool), debt);
lendingPool.repay(debt);
vm.stopPrank;
}
​
function testWithdrawReceivesLess() external {
// depositor withdraws from stability pool
vm.startPrank(depositor);
uint256 deTokenBalance = deToken.balanceOf(depositor);
deToken.approve(address(stabilityPool), deTokenBalance);
stabilityPool.withdraw(deTokenBalance);
uint256 rtokenBalance = rtoken.balanceOf(depositor);
rtoken.approve(address(lendingPool), rtokenBalance);
// depositor withdraws from lending pool
lendingPool.withdraw(rtokenBalance);
// depositor is left with 9_012.34567 USD
emit log_named_decimal_uint("crvUSD amount", asset.balanceOf(depositor), 18);
// depositor lost fund after withdrawing
assertLt(asset.balanceOf(depositor), userAssetAmount);
assertEq(rtoken.balanceOf(depositor), 0);
assertEq(deToken.balanceOf(depositor), 0);
assertEq(raacToken.balanceOf(depositor), 0);
vm.stopPrank();
}
​
function testDoubleSpend() external {
// depositor withdraws from stability pool
vm.startPrank(depositor);
uint256 deTokenBalance = deToken.balanceOf(depositor);
deToken.approve(address(stabilityPool), deTokenBalance);
stabilityPool.withdraw(deTokenBalance);
uint256 rtokenBalance = rtoken.balanceOf(depositor);
// depositor transfers all balance to other user
address otherUser = makeAddr("otherUser");
rtoken.transfer(otherUser, rtokenBalance);
// depositor's balance is still greater than 0
assertGt(rtoken.balanceOf(depositor), 0);
vm.stopPrank();
}
}
​

Console Output:

[PASS] testDoubleSpend() (gas: 195040)
[PASS] testWithdrawReceivesLess() (gas: 292654)
Logs:
crvUSD amount: 9012.345679012345679012

Impact

  • RToken can be double spent.

  • Depositors lose fund in spite of being depositors.

Tools Used

Manual Review, Foundry

Recommendations

NOTE

Inconsistency between transfer and transferFrom calculation is not important. Because scaling calculation should not be done there in the first place.

Correct fix would be removing double scaling altogether.

diff --git a/contracts/core/tokens/RToken.sol b/contracts/core/tokens/RToken.sol
index 25f6324..e9a5ea3 100644
--- a/contracts/core/tokens/RToken.sol
+++ b/contracts/core/tokens/RToken.sol
@@ -210,8 +210,7 @@ contract RToken is ERC20, ERC20Permit, IRToken, Ownable {
* @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);
+ return super.transfer(recipient, amount);
}
/**
@@ -221,8 +220,7 @@ contract RToken is ERC20, ERC20Permit, IRToken, Ownable {
* @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);
+ return super.transferFrom(sender, recipient, amount);
}
/**
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 2 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 2 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.