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

Core protocol functions will be DoS'd because `_withdrawFromVault` will revert, as `msg.sender` is incorrectly deemed the curve vault owner

056Security

Summary

In the LendingPool, two mechanisms ensure that the pool's liquidity stays within safe margins for protocol users. These functions are _rebalanceLiquidity and _ensureLiquidity. The problem arises whenever these functions call the crvVault using withdrawFromVault.

Vulnerability Details

The code suggests that the owner of the tokens inside the crvVault is the msg.sender when in reality, it is the LendingPool contract. If we look at the source code for the curve vault we can see that indeed the third argument sent to withdraw is the assets' owner:

https://github.com/curvefi/scrvusd/blob/95a120847c7a2901cea5256ba081494e18ea5315/contracts/yearn/VaultV3.vy#L1830

@external
@nonreentrant("lock")
def withdraw(
assets: uint256,
receiver: address,
owner: address,
max_loss: uint256 = 0,
strategies: DynArray[address, MAX_QUEUE] = []
) -> uint256:
"""
@notice Withdraw an amount of asset to `receiver` burning `owner`s shares.
@dev The default behavior is to not allow any loss.
@param assets The amount of asset to withdraw.
@param receiver The address to receive the assets.
@param owner The address who's shares are being burnt.
@param max_loss Optional amount of acceptable loss in Basis Points.
@param strategies Optional array of strategies to withdraw from.
@return The amount of shares actually burnt.
"""
shares: uint256 = self._convert_to_shares(assets, Rounding.ROUND_UP)
self._redeem(msg.sender, receiver, owner, assets, shares, max_loss, strategies)
return shares

As a result, whenever operations such as deposit, withdrawal, or borrow are executed, the protocol may revert.

function _ensureLiquidity(uint256 amount) internal {
// if curve vault is not set, do nothing
if (address(curveVault) == address(0)) {
return;
}
​
uint256 availableLiquidity = IERC20(reserve.reserveAssetAddress).balanceOf(reserve.reserveRTokenAddress);
​
if (availableLiquidity < amount) {
uint256 requiredAmount = amount - availableLiquidity;
// Withdraw required amount from the Curve vault
_withdrawFromVault(requiredAmount);
}
}
​
/**
* @notice Rebalances liquidity between the buffer and the Curve vault to maintain the desired buffer ratio
*/
function _rebalanceLiquidity() internal {
// if curve vault is not set, do nothing
if (address(curveVault) == address(0)) {
return;
}
​
uint256 totalDeposits = reserve.totalLiquidity; // Total liquidity in the system
uint256 desiredBuffer = totalDeposits.percentMul(liquidityBufferRatio); //20e18
uint256 currentBuffer = IERC20(reserve.reserveAssetAddress).balanceOf(reserve.reserveRTokenAddress);
​
if (currentBuffer > desiredBuffer) {
uint256 excess = currentBuffer - desiredBuffer;
// Deposit excess into the Curve vault
_depositIntoVault(excess);
} else if (currentBuffer < desiredBuffer) {
uint256 shortage = desiredBuffer - currentBuffer;
// Withdraw shortage from the Curve vault
_withdrawFromVault(shortage);
}
​
emit LiquidityRebalanced(currentBuffer, totalVaultDeposits);
}
​

Whenever the (availableLiquidity < amount) or (currentBuffer < desiredBuffer) the protocol will revert

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

Impact

DoS of key protocol functions

Tools Used

Manual review

Recommendations

Switch msg.sender to address(this) in the _withdrawFromVault(...) function.

Updates

Lead Judging Commences

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