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

broken transfer logic in `RToken` cause the transferred amount is smaller than intended

farismaulana

Summary

RToken::_updateis overrided so that mintand burnfunction can receive normalized amount of token that later be converted to scaled amount. But this have unintended behaviour when paired by transferand transferFromwhereas these two function also use the overrided update function.

Vulnerability Details

both transferand transferFromfunction would scale the amountparams given to them by dividing this with the index LendingPool::getNormalizedIncomeso the intention was to handle transfer amountin term of normalized amount but later it would transfer the scaled amount.

RToken.sol#L207-L226

/**
* @dev Overrides the ERC20 transfer function to use scaled amounts
* @param recipient The recipient address
* @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);
}
​
/**
* @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);
}

because the passed parameter is scaledAmountwhich already divided by the index then it get passed into the parent ERC20 function super.transferand super.transferFromwhich later would call parent _transfer function with said scaledAmount. but here is the issue, the _transfer function would later call the _update function but in this context the _update function is overrided.

function _transfer(address from, address to, uint256 value) internal {
if (from == address(0)) {
revert ERC20InvalidSender(address(0));
}
if (to == address(0)) {
revert ERC20InvalidReceiver(address(0));
}
@> _update(from, to, value);
}
​

we know that the update function is overrided with this logic:
RToken.sol#L307-L311

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);
}

the amount in _update function is divided again by the current index of LendingPool::getNormalizedIncome, making the original amount divided twice by the current index.

to run PoC provided, use the detailed step below:

  1. Run npm i --save-dev @nomicfoundation/hardhat-foundry in the terminal to install the hardhat-foundry plugin.

  2. Add require("@nomicfoundation/hardhat-foundry"); to the top of the hardhat.config.cjs file.

  3. Run npx hardhat init-foundry in the terminal.

  4. rename ReserveLibraryMock.solto ReserveLibraryMock.sol.bakinside test/unit/librariesfolder so it does not throw error.

  5. Create a file “Test.t.sol” in the “test/” directory and paste the provided PoC.

// SPDX-License-Identifier: MIT
​
pragma solidity ^0.8.19;
​
import "forge-std/Test.sol";
import "../contracts/core/tokens/RToken.sol";
import "../contracts/mocks/core/tokens/ERC20Mock.sol";
import "../contracts/libraries/math/WadRayMath.sol";
​
contract MockLendingPool {
RToken public rToken;
​
uint256 normalizedIncome;
​
constructor(address _rToken) {
rToken = RToken(_rToken);
normalizedIncome = 1e27;
}
​
function setNormalizedIncome(uint256 _normalizedIncome) external {
normalizedIncome = _normalizedIncome;
}
​
function getNormalizedIncome() external view returns (uint256) {
return normalizedIncome;
}
​
function mockMint(address caller, address onBehalfOf, uint256 amount) external returns (bool, uint256, uint256, uint256) {
return rToken.mint(caller, onBehalfOf, amount, normalizedIncome);
}
}
​
contract TestRToken is Test {
ERC20Mock public assetToken;
RToken public rToken;
MockLendingPool public lendingPool;
​
using WadRayMath for uint256;
​
address owner = makeAddr("owner");
address borrower = makeAddr("borrower");
address lender = makeAddr("lender");
address alice = makeAddr("alice");
​
function setUp() public {
vm.startPrank(owner);
assetToken = new ERC20Mock("Asset Token", "ATKN");
rToken = new RToken("RToken", "RTKN", owner, address(assetToken));
lendingPool = new MockLendingPool(address(rToken));
rToken.setReservePool(address(lendingPool));
​
// mint asset tokens
assetToken.mint(address(borrower), 1000 ether);
assetToken.mint(address(lender), 1000 ether);
assetToken.mint(address(alice), 1000 ether);
vm.stopPrank();
}
​
function test_transferLogic_broken() public {
// mint some RToken with index = 1.1e27
lendingPool.setNormalizedIncome(1.1e27);
uint256 amount = 100 ether;
lendingPool.mockMint(address(lendingPool), lender, amount);
​
// update index to 1.2 e27
lendingPool.setNormalizedIncome(1.2e27);
​
// transfer RToken from lender to alice
vm.startPrank(lender);
uint256 scaledTransferredAmount = amount.rayDiv(lendingPool.getNormalizedIncome());
rToken.transfer(alice, amount);
vm.stopPrank();
​
// check balances
console.log("after transfer:");
console.log("lender scaled balance: ", rToken.scaledBalanceOf(lender));
console.log("alice scaled balance: ", rToken.scaledBalanceOf(alice));
assertEq(rToken.scaledBalanceOf(alice), scaledTransferredAmount);
}
}
​

after running the command forge t --mt test_transferLogic -vvthe assert would fail:

Ran 1 test for test/RToken.t.sol:TestRToken
[FAIL: assertion failed: 69444444444444444444 != 83333333333333333333] test_transferLogic_broken() (gas: 142958)
Logs:
after transfer:
lender scaled balance: 21464646464646464647
alice scaled balance: 69444444444444444444
​
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1.71ms (372.39µs CPU time)
​
Ran 1 test suite in 11.94ms (1.71ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
​
Failing tests:
Encountered 1 failing test in test/RToken.t.sol:TestRToken
[FAIL: assertion failed: 69444444444444444444 != 83333333333333333333] test_transferLogic_broken() (gas: 142958)
​
Encountered a total of 1 failing tests, 0 tests succeeded

the transfer above supposed to transfer only 100 ether of underlying asset which equivalent of ~83 ether of RToken but instead it transfer ~69 ether of RToken, smaller amount than intended.

Impact

this would lead to incorrect/smaller amount of transfer when dealing with RToken. Although no funds loss if user just use it for simple transfer, this is not such a case where RToken later would be used in DeFI ecosystem (DEX, other lending-borrowing protocol) because of this breaking the amount would make it impossible for RToken to be used in DeFI like adding liquidity, swap etc.

Tools Used

manual review

Recommendations

My recommended mitigation for transfer: consider to transfer as the amount specified and not scaling it. this would makes the RToken can be used in other DeFI ecosystem in general. This is recommended because in the protocol there are no transfer / transferFrom use of RToken.

If protocol wants to keep the current implementation transfer logic, consider remove the first index divide operation so it transfer the correct scaled amount.

Updates

Lead Judging Commences

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