Summary
In LendingPool, the interaction with the Curve vault is wrongly implemented and it withdraws assets to the wrong address.
Vulnerability Details
Throughout the protocol, the _ensureLiquidity and _rebalanceLiquidity functions are called at different places, also explained by their natspec:
* @notice Internal function to ensure sufficient liquidity is available for withdrawals or borrowing
* @param amount The amount required
*/
function _ensureLiquidity(uint256 amount) internal {
if (address(curveVault) == address(0)) {
return;
}
uint256 availableLiquidity = IERC20(reserve.reserveAssetAddress).balanceOf(reserve.reserveRTokenAddress);
if (availableLiquidity < amount) {
uint256 requiredAmount = amount - availableLiquidity;
_withdrawFromVault(requiredAmount);
}
}
* @notice Rebalances liquidity between the buffer and the Curve vault to maintain the desired buffer ratio
*/
function _rebalanceLiquidity() internal {
if (address(curveVault) == address(0)) {
return;
}
uint256 totalDeposits = reserve.totalLiquidity;
uint256 desiredBuffer = totalDeposits.percentMul(liquidityBufferRatio);
uint256 currentBuffer = IERC20(reserve.reserveAssetAddress).balanceOf(reserve.reserveRTokenAddress);
if (currentBuffer > desiredBuffer) {
uint256 excess = currentBuffer - desiredBuffer;
_depositIntoVault(excess);
} else if (currentBuffer < desiredBuffer) {
uint256 shortage = desiredBuffer - currentBuffer;
_withdrawFromVault(shortage);
}
emit LiquidityRebalanced(currentBuffer, totalVaultDeposits);
}
According to the protocol's needs, it deposits or withdraws from the Curve vault using these functions:
function _depositIntoVault(uint256 amount) internal {
IERC20(reserve.reserveAssetAddress).approve(address(curveVault), amount);
curveVault.deposit(amount, address(this));
totalVaultDeposits += amount;
}
function _withdrawFromVault(uint256 amount) internal {
curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0));
totalVaultDeposits -= amount;
}
As we can see by the vault's interface, during deposit the receiver of the shares is address(this):
ICurveCrvUSDVault.sol
* @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);
However, when withdrawing, the protocol enters address(this) as the receiver of the assets and msg.sender as the owner of the assets:
ICurveCrvUSDVault.sol
* @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);
The owner of the shares should be address(this) as this is the address which holds the shares. Also, the receiver should be the rToken address as this is the address which holds all the assets. For example, as we can see in the withdraw function which calls _ensureLiquidity, it calls the ReserveLibrary.withdraw(..) function which in turn the rToken.burn(..) function which transfers the underlying asset amount to the caller:
LendingPool.sol
function withdraw(uint256 amount) external nonReentrant whenNotPaused onlyValidAmount(amount) {
.
.
_ensureLiquidity(amount);
@> (uint256 amountWithdrawn, uint256 amountScaled, uint256 amountUnderlying) = ReserveLibrary.withdraw(
reserve,
rateData,
amount,
msg.sender
);
.
.
ReserveLibrary.sol
function withdraw(ReserveData storage reserve, ReserveRateData storage rateData, uint256 amount, address recipient)
internal
returns (uint256 amountWithdrawn, uint256 amountScaled, uint256 amountUnderlying)
{
if (amount < 1) revert InvalidAmount();
updateReserveInterests(reserve, rateData);
@> (uint256 burnedScaledAmount, uint256 newTotalSupply, uint256 amountUnderlying) = IRToken(
reserve.reserveRTokenAddress
).burn(
recipient,
recipient,
amount,
reserve.liquidityIndex
);
.
.
rToken.sol
function burn(address from, address receiverOfUnderlying, uint256 amount, uint256 index)
external
override
onlyReservePool
returns (uint256, uint256, uint256)
{
.
.
_burn(from, amount.toUint128());
if (receiverOfUnderlying != address(this)) {
@> IERC20(_assetAddress).safeTransfer(receiverOfUnderlying, amount);
}
Impact
The protocol cannot withdraw funds from the Curve vault, only deposit, as it is called the withdraw function with wrong owner of the shares and also wrong receiver if the call would go through.
Tools Used
Manual review
Recommendations
Fix the _withdrawFromVault like this:
function _withdrawFromVault(uint256 amount) internal {
-- curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0));
++ curveVault.withdraw(amount, address(rToken), address(this), 0, new address[](0));
totalVaultDeposits -= amount;
}
