Submission Details
Severity:
Incorrect Asset Ownership in `LendingPool` Causing Deposit Failures
Summary
The deposit function in the LendingPool contract transfers reserveAsset from the user to reserveRTokenAddress. However, during the liquidity rebalancing process, _depositIntoVault attempts to deposit these assets into the curveVault, leading to a transaction failure due to insufficient funds.
function deposit(uint256 amount) external nonReentrant whenNotPaused onlyValidAmount(amount) {
// Update the reserve state before the deposit
ReserveLibrary.updateReserveState(reserve, rateData);
// Perform the deposit through ReserveLibrary
@> uint256 mintedAmount = ReserveLibrary.deposit(reserve, rateData, amount, msg.sender);
// Rebalance liquidity after deposit
@> _rebalanceLiquidity();
emit Deposit(msg.sender, amount, mintedAmount);
}
Vulnerability Details
The deposit function transfers reserveAsset from the user to reserveRTokenAddress via the ReseveLibrary.deposity function:
function deposit(
ReserveData storage reserve,
ReserveRateData storage rateData,
uint256 amount,
address depositor
) internal returns (uint256 amountMinted) {
[...]
// Transfer asset from caller to the RToken contract
@> IERC20(reserve.reserveAssetAddress).safeTransferFrom(
msg.sender, // from
reserve.reserveRTokenAddress, // to
amount // amount
);
[...]
}
It then calls _rebalanceLiquidity. If address(curveVault) != address(0), _rebalanceLiquidity may call _depositIntoVault(excess).
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);
} else if (currentBuffer < desiredBuffer) {
[...]
}
[...]
}
Which internally executes:
function _depositIntoVault(uint256 amount) internal {
IERC20(reserve.reserveAssetAddress).approve(address(curveVault), amount);
@> curveVault.deposit(amount, address(this));
[...]
}
The issue arises because the LendingPool contract does not hold the reserveAsset. Instead, the assets are held by reserveRTokenAddress, meaning that when _depositIntoVault attempts to deposit into curveVault, the transaction will revert due to insufficient funds.
PoC
Add a mock for the CRV USD vault based on the provided interface (ICurveCrvUSDVault)
// SPDX-License-Identifier: MIT
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "../../../interfaces/curve/ICurveCrvUSDVault.sol";
import "hardhat/console.sol";
/**
* @title ICrvUSDToken
* @notice Interface for the Curve USD (crvUSD) token contract
*/
interface ICrvUSDToken is IERC20 {
/**
* @notice Returns the current minter address
* @return The address of the minter
*/
function minter() external view returns (address);
/**
* @notice Mints new crvUSD tokens
* @dev Allows anyone to mint (according to the contract logic)
* @param to The address to receive the minted tokens
* @param amount The amount of tokens to mint
*/
function mint(address to, uint256 amount) external;
/**
* @notice Sets a new minter address
* @dev Can only be called by the contract owner
* @param _minter The new minter address
*/
function setMinter(address _minter) external;
/**
* @notice Burns tokens from the caller's balance
* @param amount The amount of tokens to burn
*/
function burn(uint256 amount) external;
/**
* @notice Burns tokens from another account, using an allowance mechanism
* @dev Reduces the caller's allowance on the target account by the burned amount
* @param account The account from which tokens will be burned
* @param amount The amount of tokens to burn
*/
function burnFrom(address account, uint256 amount) external;
}
contract MockCurveCrvUSDVault is ICurveCrvUSDVault, ReentrancyGuard, Ownable {
ICrvUSDToken private immutable _asset;
uint256 private _totalAssets;
uint256 private _totalDebt;
uint256 private _totalIdle;
uint256 private _totalShares;
bool private _shutdown;
mapping(address => uint256) private _balances;
mapping(address => StrategyParams) private _strategies;
constructor(address asset_) Ownable(msg.sender) {
_asset = ICrvUSDToken(asset_);
console.log("_asset", address(_asset));
}
/* ========== CORE FUNCTIONS ========== */
function deposit(uint256 assets, address receiver) external override nonReentrant returns (uint256 shares) {
if (_shutdown) revert VaultShutdown();
if (assets == 0) revert DepositLimitExceeded();
shares = assets; // Simple 1:1 shares-to-assets model
_totalAssets += assets;
_totalIdle += assets;
_totalShares += shares;
_balances[receiver] += shares;
console.log("vault assets", assets);
_asset.transferFrom(msg.sender, address(this), assets);
emit Deposit(msg.sender, receiver, assets, shares);
}
function withdraw(
uint256 assets,
address receiver,
address owner,
uint256 maxLoss,
address[] calldata strategies
) external override nonReentrant returns (uint256 shares) {
if (_shutdown) revert VaultShutdown();
if (assets > _totalAssets) revert InsufficientAvailableAssets();
shares = assets; // 1:1 model
if (_balances[owner] < shares) revert InsufficientAvailableAssets();
_balances[owner] -= shares;
_totalAssets -= assets;
_totalIdle -= assets;
_totalShares -= shares;
_asset.transfer(receiver, assets);
emit Withdraw(msg.sender, receiver, owner, assets, shares);
}
/* ========== VIEW FUNCTIONS ========== */
function asset() external view override returns (address) {
return address(_asset);
}
function totalAssets() external view override returns (uint256) {
return _totalAssets;
}
function pricePerShare() external view override returns (uint256) {
return _totalShares == 0 ? 1e18 : (_totalAssets * 1e18) / _totalShares;
}
function totalIdle() external view override returns (uint256) {
return _totalIdle;
}
function totalDebt() external view override returns (uint256) {
return _totalDebt;
}
function isShutdown() external view override returns (bool) {
return _shutdown;
}
/* ========== ADMIN FUNCTIONS ========== */
function shutdownVault() external onlyOwner {
_shutdown = true;
}
function activateVault() external onlyOwner {
_shutdown = false;
}
}
Then, in test/unit/core/pools/LendingPool/LendingPool.test.js, add the following changes to the beforeEach block
describe("LendingPool", function () {
let owner, user1, user2, user3;
let crvusd, raacNFT, raacHousePrices, stabilityPool, raacFCL, raacVault;
let lendingPool, rToken, debtToken;
let deployer;
let token;
+ let crvVault;
beforeEach(async function () {
[owner, user1, user2, user3] = await ethers.getSigners();
const CrvUSDToken = await ethers.getContractFactory("crvUSDToken");
crvusd = await CrvUSDToken.deploy(owner.address);
await crvusd.setMinter(owner.address);
token = crvusd;
+ const CrvUSDVault = await ethers.getContractFactory("MockCurveCrvUSDVault");
+ crvVault = await CrvUSDVault.deploy(crvusd.target);
const RAACHousePrices = await ethers.getContractFactory("RAACHousePrices");
raacHousePrices = await RAACHousePrices.deploy(owner.address);
const RAACNFT = await ethers.getContractFactory("RAACNFT");
raacNFT = await RAACNFT.deploy(crvusd.target, raacHousePrices.target, owner.address);
stabilityPool = { target: owner.address };
const RToken = await ethers.getContractFactory("RToken");
rToken = await RToken.deploy("RToken", "RToken", owner.address, crvusd.target);
const DebtToken = await ethers.getContractFactory("DebtToken");
debtToken = await DebtToken.deploy("DebtToken", "DT", owner.address);
const initialPrimeRate = ethers.parseUnits("0.1", 27);
const LendingPool = await ethers.getContractFactory("LendingPool");
lendingPool = await LendingPool.deploy(
crvusd.target,
rToken.target,
debtToken.target,
raacNFT.target,
raacHousePrices.target,
initialPrimeRate
);
+ await lendingPool.setCurveVault(crvVault.target);
await rToken.setReservePool(lendingPool.target);
await debtToken.setReservePool(lendingPool.target);
await rToken.transferOwnership(lendingPool.target);
await debtToken.transferOwnership(lendingPool.target);
const mintAmount = ethers.parseEther("1000");
await crvusd.mint(user1.address, mintAmount);
await crvusd.mint(user3.address, mintAmount);
const mintAmount2 = ethers.parseEther("10000");
await crvusd.mint(user2.address, mintAmount2);
await crvusd.connect(user1).approve(lendingPool.target, mintAmount);
await crvusd.connect(user2).approve(lendingPool.target, mintAmount);
await crvusd.connect(user3).approve(lendingPool.target, mintAmount);
await raacHousePrices.setOracle(owner.address);
// FIXME: we are using price oracle and therefore the price should be changed from the oracle.
await raacHousePrices.setHousePrice(1, ethers.parseEther("100"));
await ethers.provider.send("evm_mine", []);
const housePrice = await raacHousePrices.tokenToHousePrice(1);
const raacHpAddress = await raacNFT.raac_hp();
const priceFromNFT = await raacNFT.getHousePrice(1);
const tokenId = 1;
const amountToPay = ethers.parseEther("100");
await token.mint(user1.address, amountToPay);
await token.connect(user1).approve(raacNFT.target, amountToPay);
await raacNFT.connect(user1).mint(tokenId, amountToPay);
const depositAmount = ethers.parseEther("1000");
await crvusd.connect(user2).approve(lendingPool.target, depositAmount);
await lendingPool.connect(user2).deposit(depositAmount);
await ethers.provider.send("evm_mine", []);
expect(await crvusd.balanceOf(rToken.target)).to.equal(ethers.parseEther("1000"));
});
Finally run the LendingPool test suit (npm run test:unit:pools:lendingpool) and and wait for the test to fail with:
1) LendingPool
"before each" hook for "should allow user to withdraw crvUSD by burning rToken":
Error: VM Exception while processing transaction: reverted with custom error 'ERC20InsufficientBalance("0xa513E6E4b8f2a923D98304ec87F64353C4D5C853", 0, 800000000000000000000)'
Impact
Deposits will fail when _depositIntoVault is executed.
Tools Used
Manual review
Recommendations
Ensure that
_depositIntoVaultpulls assets fromreserveRTokenAddressinstead of assuming they are held by the LendingPool contract.Consider adding a mechanism to transfer funds from
reserveRTokenAddressto LendingPool before interacting withcurveVault.
Updates
Lead Judging Commences
inallhonesty 3 months ago
Submission Judgement Published Assigned finding tags:
LendingPool::_depositIntoVault and _withdrawFromVault don't transfer tokens between RToken and LendingPool, breaking Curve vault interactions
inallhonesty 3 months ago
Submission Judgement Published Assigned finding tags:
LendingPool::_depositIntoVault and _withdrawFromVault don't transfer tokens between RToken and LendingPool, breaking Curve vault interactions
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.