Submission Details
Severity:
Permanent Loss of Curve shares of some users to the lending pool and a DOS to other users calling withdrawals/deposits/borrow
Summary
The call to rebalance the pool is flawed and can lead to a DOS of user funds and also some users with shares in the Curvevault will permanently loss their tokens.
Vulnerability Details
During the rebalancing of the vault funds when the curve vault is utilized, the vault tries to rebalance the vault
1.
// CORE FUNCTIONS
/**
* @notice Allows a user to deposit reserve assets and receive RTokens
* @param amount The amount of reserve assets to deposit
*/
function deposit(uint256 amount) external nonReentrant whenNotPaused onlyValidAmount(amount) {
// Update the reserve state before the deposit
ReserveLibrary.updateReserveState(reserve, rateData); // this are just repeitions they are being done regarless in the reserve library
// Perform the deposit through ReserveLibrary
uint256 mintedAmount = ReserveLibrary.deposit(reserve, rateData, amount, msg.sender);
@audit>> // Rebalance liquidity after deposit
@audit>> _rebalanceLiquidity();
emit Deposit(msg.sender, amount, mintedAmount);
}
2.
/**
* @notice Allows a user to withdraw reserve assets by burning RTokens
* @param amount The amount of reserve assets to withdraw
*/
function withdraw(uint256 amount) external nonReentrant whenNotPaused onlyValidAmount(amount) { @audit>>
if (withdrawalsPaused) revert WithdrawalsArePaused();
// Update the reserve state before the withdrawal
ReserveLibrary.updateReserveState(reserve, rateData); // will this not be don ein the reserve library, we don't need this on the outside na
// Ensure sufficient liquidity is available
_ensureLiquidity(amount);
// Perform the withdrawal through ReserveLibrary
(uint256 amountWithdrawn, uint256 amountScaled, uint256 amountUnderlying) = ReserveLibrary.withdraw(
reserve, // ReserveData storage
rateData, // ReserveRateData storage
amount, // Amount to withdraw
msg.sender // Recipient
);
@audit>> // Rebalance liquidity after withdrawal
@audit>> _rebalanceLiquidity();
emit Withdraw(msg.sender, amountWithdrawn);
}
3.
/**
* @notice Allows a user to borrow reserve assets using their NFT collateral
* @param amount The amount of reserve assets to borrow
*/
function borrow(uint256 amount) external nonReentrant whenNotPaused onlyValidAmount(amount) {
if (isUnderLiquidation[msg.sender]) revert CannotBorrowUnderLiquidation();
UserData storage user = userData[msg.sender];
uint256 collateralValue = getUserCollateralValue(msg.sender);
if (collateralValue == 0) revert NoCollateral();
// Update reserve state before borrowing
ReserveLibrary.updateReserveState(reserve, rateData);
// Ensure sufficient liquidity is available
_ensureLiquidity(amount);
// Fetch user's total debt after borrowing
uint256 userTotalDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex) + amount;
// Ensure the user has enough collateral to cover the new debt
if (collateralValue < userTotalDebt.percentMul(liquidationThreshold)) { /
revert NotEnoughCollateralToBorrow();
}
// Update user's scaled debt balance
uint256 scaledAmount = amount.rayDiv(reserve.usageIndex);
// Mint DebtTokens to the user (scaled amount)
(bool isFirstMint, uint256 amountMinted, uint256 newTotalSupply) = IDebtToken(reserve.reserveDebtTokenAddress).mint(msg.sender, msg.sender, amount, reserve.usageIndex);
// Transfer borrowed amount to user
IRToken(reserve.reserveRTokenAddress).transferAsset(msg.sender, amount);
user.scaledDebtBalance += scaledAmount;
// reserve.totalUsage += amount;
reserve.totalUsage = newTotalSupply;
// Update liquidity and interest rates
ReserveLibrary.updateInterestRatesAndLiquidity(reserve, rateData, 0, amount);
@audit>> // Rebalance liquidity after borrowing
@audit>> _rebalanceLiquidity();
emit Borrow(msg.sender, amount);
}
If the Contract needs funds to rebalance the vault supplies
/**
* @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);
uint256 currentBuffer = IERC20(reserve.reserveAssetAddress).balanceOf(reserve.reserveRTokenAddress);
if (currentBuffer > desiredBuffer) {
uint256 excess = currentBuffer - desiredBuffer;
// Deposit excess into the Curve vault
_depositIntoVault(excess);
@audit>> } else if (currentBuffer < desiredBuffer) {
@audit>> uint256 shortage = desiredBuffer - currentBuffer;
@audit>> // Withdraw shortage from the Curve vault
@audit>> _withdrawFromVault(shortage);
}
emit LiquidityRebalanced(currentBuffer, totalVaultDeposits);
}
This call is wrongly implemented
/**
* @notice Internal function to withdraw liquidity from the Curve vault
* @param amount The amount to withdraw
*/
function _withdrawFromVault(uint256 amount) internal {
@audit >> curveVault.withdraw(amount, address(this), @audit >> msg.sender, 0, new address[](0)); //should owner not be address(this)????
totalVaultDeposits -= amount;
}
SEE, THE OWNER OF the shares to be burnt should be this contract address(this) not msg.sender ( user who wants to deposit, withdraw or borrow)
/**
* @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,
@AUDIT>> address owner,
uint256 maxLoss,
address[] calldata strategies
) external returns (uint256 shares);
If the user who is interacting has Vault share after interacting with the curve vault outside this contract his shares will be lost and his assets sent to address(this).
if the user has no interaction with the vault outside this contract this call will revert.
Knowing fully well that when we rebalance we deposit tokens and send shares to address(this) has the custodian, calling to burn from the user is a flaw.
/**
* @notice Internal function to deposit liquidity into the Curve vault
* @param amount The amount to deposit
*/
function _depositIntoVault(uint256 amount) internal {
IERC20(reserve.reserveAssetAddress).approve(address(curveVault), amount);
@audit>> curveVault.deposit(amount, address(this));
totalVaultDeposits += amount;
}
/* ========== CORE FUNCTIONS ========== */
/**
* @notice Deposits assets into the vault
* @param assets Amount of assets to deposit
@audit>> * @param receiver Address to receive the shares
* @return shares Amount of shares minted
*/
@audit>> function deposit(uint256 assets, address receiver) external returns (uint256 shares);
This issue also occurs when we call the ensure liquidity function
/**
* @notice Internal function to ensure sufficient liquidity is available for withdrawals or borrowing
* @param amount The amount required
*/
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
@audit>> _withdrawFromVault(requiredAmount);
}
}
Because the same fix solves for all I have decided to state all this functions in one report
Impact
Loss of funds to user with shares from the curve vault and a DOS to other functions when the user has never interacted with the curve vault.
Tools Used
Manual Review
Recommendations
Burn share from address(this) and not msg.sender
/**
* @notice Internal function to withdraw liquidity from the Curve vault
* @param amount The amount to withdraw
*/
function _withdrawFromVault(uint256 amount) internal {
-- curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0)); //should owner not be address(this)????
++ curveVault.withdraw(amount, address(this), address(this), 0, new address[](0));
totalVaultDeposits -= amount;
}
Updates
Lead Judging Commences
inallhonesty 6 months ago
Submission Judgement Published Assigned finding tags:
LendingPool::_withdrawFromVault incorrectly uses msg.sender instead of address(this) as the owner parameter, causing vault withdrawals to fail
inallhonesty 6 months ago
Submission Judgement Published Assigned finding tags:
LendingPool::_withdrawFromVault incorrectly uses msg.sender instead of address(this) as the owner parameter, causing vault withdrawals to fail
Prize pool breakdown
Total prize
77,280 USDC
nSLOC
3,86420 USDC / LOC
74,000
3,280
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.