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

Incorrect recipient in `LendingPool` curve vault withdrawals leads to loss of protocol assets

patitonar

Summary

In the LendingPool::_withdrawFromVault() function the withdrawal from Curve vault specifies msg.sender as the recipient instead of address(this), causing assets to be sent to the wrong address.

Vulnerability Details

The vulnerability exists in the _withdrawFromVault function:

function _withdrawFromVault(uint256 amount) internal {
@> curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0));
totalVaultDeposits -= amount;
}

The msg.sender parameter in the withdraw call represents the recipient of the withdrawn assets. However, this should be address(this) since the LendingPool needs to receive the assets first and then transfer them to the reserveRTokenAddress where all protocol assets are stored.

The current implementation sends the withdrawn assets directly to msg.sender (which could be any user calling withdraw/borrow functions), bypassing the protocol's asset management flow.

Impact

This vulnerability has severe implications:

  1. Protocol assets are sent directly to users instead of being properly managed through the RToken contract

  2. The accounting becomes incorrect as the assets bypass the protocol's tracking

  3. This could lead to direct loss of protocol funds as users receive more assets than they should

Tools Used

Manual review

Proof of Concept

First, a bug need to be fixed in the LendingPool::_depositIntoVault() contract to transfer the assets from RToken to LendingPool before attempting vault deposit:

function _depositIntoVault(uint256 amount) internal {
+ // Transfer assets from RToken to LendingPool first
+ IRToken(reserve.reserveRTokenAddress).transferAsset(address(this), amount);
​
IERC20(reserve.reserveAssetAddress).approve(address(curveVault), amount);
curveVault.deposit(amount, address(this));
totalVaultDeposits += amount;
}

Create a new Mock CurveVault contract that inherits from ERC4626 in the file contracts/mocks/core/vaults/CurveVaultMock.sol:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {ERC4626} from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {ICurveCrvUSDVault} from "../../../interfaces/curve/ICurveCrvUSDVault.sol";
​
contract CurveVaultMock is ERC4626 {
​
bool public depositEnabled;
bool public withdrawEnabled;
constructor(
address _asset
) ERC4626(IERC20(_asset)) ERC20("Mock Curve Vault", "mcrvVault") {
depositEnabled = true;
withdrawEnabled = true;
}
​
function enableDeposits(bool _depositEnabled) external {
depositEnabled = _depositEnabled;
}
​
function enableWithdrawals(bool _withdrawEnabled) external {
withdrawEnabled = _withdrawEnabled;
}
​
/**
* @notice Withdraws assets from the vault
* @param amount The amount of assets to withdraw
* @param owner The owner of the shares
* @param recipient The address to receive the assets
* @param maxLoss The maximum acceptable loss (not used in mock)
* @param strategies Array of strategies (not used in mock)
*/
function withdraw(
uint256 amount,
address owner,
address recipient,
uint256 maxLoss,
address[] calldata strategies
) external returns (uint256) {
// Mock implementation ignores maxLoss and strategies parameters
return super.withdraw(amount, recipient, owner);
}
​
/**
* @notice Returns the maximum amount that can be deposited
* @param owner The address to check deposit limit for
*/
function maxDeposit(
address owner
) public view override(ERC4626) returns (uint256) {
if (!depositEnabled) return 0;
return super.maxDeposit(owner);
}
​
function maxWithdraw(address owner) public view virtual override returns (uint256) {
if (!withdrawEnabled) return 0;
return super.maxWithdraw(owner);
}
​
function withdraw(
uint256 assets,
address receiver,
address owner
) public virtual override returns (uint256) {
// Mock implementation ignores maxLoss and strategies parameters
return super.withdraw(assets, receiver, owner);
}
}

Add the following test case to the test/unit/core/pools/LendingPool/LendingPool.test.js file:

describe("Curve Vault", function () {
let curveVault;
​
beforeEach(async function () {
const CurveVault = await ethers.getContractFactory("CurveVaultMock");
curveVault = await CurveVault.deploy(crvusd.target);
await lendingPool.setCurveVault(curveVault.target);
});
​
it("lending pool withdraw to incorrect address", async function () {
const depositAmount = ethers.parseEther("100");
await lendingPool.connect(user1).deposit(depositAmount);
​
const balanceBefore = await crvusd.balanceOf(user1.address);
​
await lendingPool.connect(user1).withdraw(depositAmount);
​
const balanceAfter = await crvusd.balanceOf(user1.address);
expect(balanceAfter).to.gt(balanceBefore + depositAmount); // Should be equal but it is bigger due to all withdrawn from vault went to user1
});
});

Recommendations

Modify the _withdrawFromVault function to use the correct recipient and transfer flow:

function _withdrawFromVault(uint256 amount) internal {
- curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0));
+ curveVault.withdraw(amount, address(this), address(this), 0, new address[](0));
+ // Transfer to RToken where protocol assets are stored
+ IERC20(reserve.reserveAssetAddress).transfer(reserve.reserveRTokenAddress, amount);
totalVaultDeposits -= amount;
}
Updates

Lead Judging Commences

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

LendingPool::_depositIntoVault and _withdrawFromVault don't transfer tokens between RToken and LendingPool, breaking Curve vault interactions

LendingPool::_withdrawFromVault incorrectly uses msg.sender instead of address(this) as the owner parameter, causing vault withdrawals to fail

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

LendingPool::_depositIntoVault and _withdrawFromVault don't transfer tokens between RToken and LendingPool, breaking Curve vault interactions

LendingPool::_withdrawFromVault incorrectly uses msg.sender instead of address(this) as the owner parameter, causing vault withdrawals to fail

Support

FAQs

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