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

`LendingPool::_withdrawFromVault` would revert due to incorrect owner parameter

tejaswarambhe

Summary

The LendingPool::_withdrawFromVault incorrectly calls the curveVault with owner of shares as msg.sender, which would revert a legitimate LendingPool::withdraw, LendingPool::deposit or LendingPool::borrow transaction.

Vulnerability Details

The LendingPool::_withdrawFromVault function is being used inside the LendingPool::_ensureLiquidity and LendingPool::_rebalanceLiquidity to maintain the enough liquidity in the LendingPool contract.

However, the _withdrawFromVault has a critical parameter of owner, which is currently being passed as msg.sender instead of address(this). The depositor of the crvUSD is the contract itself which can be verified inside the _depositIntoVault function:

function _depositIntoVault(uint256 amount) internal {
IERC20(reserve.reserveAssetAddress).approve(address(curveVault), amount);
curveVault.deposit(amount, address(this)); <<@ - // The depositor here is the contract itself
totalVaultDeposits += amount;
}

But in the case of _withdrawFromVault:

function _withdrawFromVault(uint256 amount) internal {
curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0)); <<@ - // owner of shares is being passed as msg.sender
totalVaultDeposits -= amount;
}

Which implies that the user should be fulfilling the withdraw instead of the LendingPool contract.

Impact

  1. The LendingPool::_rebalanceLiquidity and LendingPool::_ensureLiquidity would revert in case of deficit being withdrawn, which breaks the LendingPool::withdraw, LendingPool::deposit and LendingPool::borrow functions, rendering the contract useless.

  2. The funds stuck cannot be withdrawn under any circumstance as the LendingPool contact in itself is non-upgradable.

  3. Loss of funds for users who might coincidently have deposited crvUSD in the vault on a individual level.

Tools Used

Manual Review

Recommendations

It is recommended to replace msg.sender with address(this) in place of the owner parameter:

function _withdrawFromVault(uint256 amount) internal {
curveVault.withdraw(amount, address(this), address(this), 0, new address[](0)); <<@ - // Fixed logic
totalVaultDeposits -= amount;
}
Updates

Lead Judging Commences

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

Give us feedback!