Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Drain of pool funds as users can be undercollateralized, due to incorrect logic in LendingPool::withdrawNFT

crunter

Summary

Due to incorrect logic, users are able to withdraw NFTs even if their collateral value falls below the allowed level. This results in users not having enough collateral with respect to their active debt, which is not an intended system behavior and results in significant losses for the protocol.

Vulnerability Details

Users should be allowed to withdraw their RAAC NFTs if and only if the remaining collateral value is enough to maintain the liquidation threshold. Upper limit of the withdrawal value is calculated based on the user remaining collateral value. The condition to authorizing the withdrawal of an NFT must be:

RemainingCollateralValue > userTotalDebt

using the liquidation threshold percentage:

RemainingCollateralValue * liquidationThreshold > userTotalDebt

or

revert if (RemainingCollateralValue * liquidationThreshold < userTotalDebt).

Currently, the logic to validate the withdrawal value in the LendingPool::withdrawNFT function is incorrect:

revert if (RemainingCollateralValue < userDebt * liquidationThreshold)

This condition allows any user to withdraw NFTs with a higher value than expected, leaving them undercollateralized and affecting the functionality and liquidity of the protocol.

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();
​
// update state
ReserveLibrary.updateReserveState(reserve, rateData);
​
// Check if withdrawal would leave user undercollateralized
uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
uint256 collateralValue = getUserCollateralValue(msg.sender);
uint256 nftValue = getNFTPrice(tokenId);
​
// @audit - incorrect validation logic
@> if (collateralValue - nftValue < userDebt.percentMul(liquidationThreshold)) {
revert WithdrawalWouldLeaveUserUnderCollateralized();
}
​
// Remove NFT from user's deposited NFTs
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);
}

Proof of Concept

Assuming that the reserve token is crvUsd and liquidationThreshold = BASE_LIQUIDATION_THRESHOLD = 80%, let's take the following example:

  • UserA has 3 RAAC NFTs deposited in the pool, which are

    • NFT1 (25,000 crvUsd)

    • NFT2 (14,000 crvUsd)

    • NFT3 (31,000 crvUsd)

  • UserA current debt is 55,000 crvUsd (78% of his collateral value)

  • He decides to withdraw his NFT1 by calling the LendingPool::withdrawNFT function (would leave him undercollateralized)

    • userDebt = 55000

    • collateralValue = getUserCollateralValue(msg.sender) = 70000

    • nftValue = getNFTPrice(NFT1) = 25000

    • (collateralValue - nftValue < userDebt.percentMul(liquidationThreshold))

      • 70000 - 25000 < 55000 * 0.8

      • (45000 < 44000) is false, then the process will continue

  • After the operation, collateral value of userA is NFT2 + NFT3 = 45,000 crvUsd, while his current debt is 55,000 crvUsd

In this way, malicious user managed to withdraw his NFT and become undercollateralized, causing losses to the protocol.

Impact

Impact: High

Likelihood: High

Tools Used

Manual Review

Recommendations

Perform a correct validation of the remaining collateral value:

> LendingPool.sol
​
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();
​
// update state
ReserveLibrary.updateReserveState(reserve, rateData);
​
// Check if withdrawal would leave user undercollateralized
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();
}
​
// Remove NFT from user's deposited NFTs
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);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

LendingPool::borrow as well as withdrawNFT() reverses collateralization check, comparing collateral < debt*0.8 instead of collateral*0.8 > debt, allowing 125% borrowing vs intended 80%

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!