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

Incorrect logic in _withdrawFromVault function

biakia

Summary

The _depositIntoVault and _withdrawFromVault functions in the LendingPool contract interact with the ICurveCrvUSDVault interface to manage liquidity. However, the _withdrawFromVault function contains incorrect parameter settings, specifically for the owner and receiver arguments in the withdraw function call. This misconfiguration could lead to improper handling of shares and assets, potentially causing liquidity management issues or loss of funds.

Vulnerability Details

In _depositIntoVault function, the deposit function of ICurveCrvUSDVault is called with amount and address(this) as arguments.

function _depositIntoVault(uint256 amount) internal {
IERC20(reserve.reserveAssetAddress).approve(address(curveVault), amount);
curveVault.deposit(amount, address(this));
totalVaultDeposits += amount;
}

Here is the comment in the deposit function of ICurveCrvUSDVault

/**
* @notice Deposits assets into the vault
* @param assets Amount of assets to deposit
* @param receiver Address to receive the shares
* @return shares Amount of shares minted
*/
function deposit(uint256 assets, address receiver) external returns (uint256 shares);

In _depositIntoVault function, address(this) is correctly set as the receiver of the shares, meaning the LendingPool contract will own the shares. This implementation is correct and aligns with the intended functionality.

In _withdrawFromVault function, the withdraw function of ICurveCrvUSDVault is called with the following arguments:

  • assets: The amount of assets to withdraw.

  • receiver: The address receiving the assets, set to address(this).

  • owner: The address owning the shares, set to msg.sender.

  • maxLoss: The maximum acceptable loss, set to 0.

  • strategies: Optional strategies to withdraw from, set to an empty array.

/**
* @notice Withdraws assets from the vault
* @param assets Amount of assets to withdraw
* @param receiver Address to receive the assets
* @param owner Owner of the shares
* @param maxLoss Maximum acceptable loss in basis points
* @param strategies Optional specific strategies to withdraw from
* @return shares Amount of shares burned
*/
function withdraw(
uint256 assets,
address receiver,
address owner,
uint256 maxLoss,
address[] calldata strategies
) external returns (uint256 shares);
function _withdrawFromVault(uint256 amount) internal {
curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0));
totalVaultDeposits -= amount;
}

The owner is set to msg.sender, which is incorrect. The shares are owned by the LendingPool contract (address(this)), not the caller (msg.sender). This could cause the withdrawal to fail, as msg.sender does not own the shares.

The receiver is set to address(this), which is incorrect. The withdrawn assets should be sent directly to the reserve.reserveRTokenAddress (the reserve token contract) to ensure proper liquidity management. Sending the assets to address(this) could result in assets being stuck in the LendingPool contract.

Impact

  • Asset Mismanagement:

    • Sending the withdrawn assets to address(this) instead of reserve.reserveRTokenAddress could result in assets being trapped in the LendingPool contract, disrupting the liquidity management system.

  • Potential Loss of Funds:

    • If the withdrawal fails or assets are mismanaged, users may be unable to withdraw their funds, leading to potential loss of trust or financial losses.

The impact is High, the likelihood is Medium, so the severity is High.

Tools Used

Manual Review

Recommendations

Consider following fix:

function _withdrawFromVault(uint256 amount) internal {
curveVault.withdraw(
amount, // Amount of assets to withdraw
reserve.reserveRTokenAddress, // Receiver of the assets
address(this), // Owner of the shares
0, // Maximum acceptable loss
new address[](0) // Optional strategies
);
totalVaultDeposits -= amount;
}
​
Updates

Lead Judging Commences

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

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::_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.