Summary
The LendingPool::_withdrawFromVault() function in LendingPool fails to account for yield generated in the Curve vault, leading to lost returns and incorrect total deposit tracking. Users cannot withdraw their proportional share of generated yield.
Vulnerability Details
In LendingPool::_withdrawFromVault() function has two critical issues:
Line 1: The withdrawal call to Curve vault doesn't account for yield:
curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0));
Line 2: The accounting simply deducts the original deposit amount:
totalVaultDeposits -= amount;
The Curve vault follows ERC4626 which means:
Yield generated stays in the vault
Withdrawals only return the exact requested amount
The protocol's accounting doesn't track yield accrual
This means any yield generated in the Curve vault becomes permanently locked in the vault and inaccessible to users.
Impact
Direct loss of yield for users - all yield generated in the Curve vault becomes inaccessible
Users only receive their principal back without any returns
Protocol cannot properly track or distribute vault yields
Tools Used
Manual Review
Proof of Concept
First, a bug need to be fixed in the LendingPool::_depositIntoVault() contract to transfer the assets from RToken to LendingPool before attempting vault deposit:
function _depositIntoVault(uint256 amount) internal {
+ // Transfer assets from RToken to LendingPool first
+ IRToken(reserve.reserveRTokenAddress).transferAsset(address(this), amount);
IERC20(reserve.reserveAssetAddress).approve(address(curveVault), amount);
curveVault.deposit(amount, address(this));
totalVaultDeposits += amount;
}
Second, a bug need to be fixed in the LendingPool::_withdrawFromVault() contract to correctly withdraw assets from the vault:
function _withdrawFromVault(uint256 amount) internal {
- curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0));
+ curveVault.withdraw(amount, address(this), address(this), 0, new address[](0));
+ // Transfer to RToken where protocol assets are stored
+ IERC20(reserve.reserveAssetAddress).transfer(reserve.reserveRTokenAddress, amount);
totalVaultDeposits -= amount;
}
Create a new Mock CurveVault contract that inherits from ERC4626 in the file contracts/mocks/core/vaults/CurveVaultMock.sol:
pragma solidity ^0.8.19;
import {ERC4626} from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {ICurveCrvUSDVault} from "../../../interfaces/curve/ICurveCrvUSDVault.sol";
contract CurveVaultMock is ERC4626 {
bool public depositEnabled;
bool public withdrawEnabled;
constructor(
address _asset
) ERC4626(IERC20(_asset)) ERC20("Mock Curve Vault", "mcrvVault") {
depositEnabled = true;
withdrawEnabled = true;
}
function enableDeposits(bool _depositEnabled) external {
depositEnabled = _depositEnabled;
}
function enableWithdrawals(bool _withdrawEnabled) external {
withdrawEnabled = _withdrawEnabled;
}
* @notice Withdraws assets from the vault
* @param amount The amount of assets to withdraw
* @param owner The owner of the shares
* @param recipient The address to receive the assets
* @param maxLoss The maximum acceptable loss (not used in mock)
* @param strategies Array of strategies (not used in mock)
*/
function withdraw(
uint256 amount,
address owner,
address recipient,
uint256 maxLoss,
address[] calldata strategies
) external returns (uint256) {
return super.withdraw(amount, recipient, owner);
}
* @notice Returns the maximum amount that can be deposited
* @param owner The address to check deposit limit for
*/
function maxDeposit(
address owner
) public view override(ERC4626) returns (uint256) {
if (!depositEnabled) return 0;
return super.maxDeposit(owner);
}
function maxWithdraw(address owner) public view virtual override returns (uint256) {
if (!withdrawEnabled) return 0;
return super.maxWithdraw(owner);
}
function withdraw(
uint256 assets,
address receiver,
address owner
) public virtual override returns (uint256) {
return super.withdraw(assets, receiver, owner);
}
}
Add the following test case to the test/unit/core/pools/LendingPool/LendingPool.test.js file:
describe("Curve Vault", function () {
let curveVault;
beforeEach(async function () {
const CurveVault = await ethers.getContractFactory("CurveVaultMock");
curveVault = await CurveVault.deploy(crvusd.target);
await lendingPool.setCurveVault(curveVault.target);
});
it("yield loss in vault withdrawals", async function () {
const depositAmount = ethers.parseEther("100");
await lendingPool.connect(user1).deposit(depositAmount);
const vaultSharesBalance = await curveVault.balanceOf(lendingPool.target);
const poolAssetsInVault = await curveVault.convertToAssets(vaultSharesBalance);
await crvusd.mint(curveVault.target, ethers.parseEther("1000"));
const poolAssetsInVaultAfterYield = await curveVault.convertToAssets(vaultSharesBalance);
expect(poolAssetsInVaultAfterYield).to.gt(poolAssetsInVault);
const poolYield = poolAssetsInVaultAfterYield - poolAssetsInVault;
expect(poolYield).to.gt(0);
await lendingPool.connect(user1).withdraw(depositAmount);
await lendingPool.connect(user2).withdraw(ethers.parseEther("1000"));
const rTokenTotalSupply = await rToken.totalSupply();
expect(rTokenTotalSupply).to.equal(0);
const vaultSharesRemaining = await curveVault.balanceOf(lendingPool.target);
const poolAssetsInVaultRemaining = await curveVault.convertToAssets(vaultSharesRemaining);
expect(vaultSharesRemaining).to.gt(0);
expect(poolAssetsInVaultRemaining).to.gt(0);
expect(poolAssetsInVaultRemaining).to.equal(poolYield);
});
});
Recommendations
Track shares instead of deposit amounts
Add yield tracking and distribution mechanism based on user share percentages
