Submission Details
Severity:
When an user calls liquidate(), s_DSCMinted mapping is not correctly updated
Summary
Liquidators are at risk of losing funds permanently. This issue could cause chain liquidations amongst users and internal accounting conflicts. Potentially render the protocol unusable.
Vulnerability Details
liquidate() does not update s_DSCMinted mapping properly for caller.
Impact
There are 2 cases with this issue.
Let’s say an Alice is below minimum health factor and Bob notices the opportunity and needs 200 DSC tokens take advantage. Before Bob is able to call liquidate(), he must deposit collateral(WETH) in order to mint just enough DSC. However, once liquidate() is complete, s_DSCMinted only updates onBehalfOf but not dscFrom. Although Bob holds 0 DSC after liquidate(), internal accounting still shows s_DSCMinted[Bob] = 200. Health Factors are calculated from s_DSCMinted .
Case 1:
Let’s assume price of ETH falls. This could cause Bob to be open to liquidation risks while he holds 0 DSC tokens and his health factor should be type(uint256).max. Bob will never be able to fully redeem his collaterals as it will trigger _revertIfHealthFactorIsBroken unless another user liquidates Bob paying his “DSC debt”. And if Bob does not redeem collaterals, all of his funds may be at risk due to price volatility. This pattern can be repeated and cause chain liquidations and render the protocol unusable.
Case 2:
In this scenario, Bob's health factor does not go below the minimum threshold due to a change in the price of ETH. However, the same issue with s_DSCMinted is present. When Bob tries to redeem his collaterals, the internal accounting issue prevents him from fully redeeming them.
See following test to unit test folder:
// SPDX-License-Identifier: MIT
import {DeployDSC} from "../../script/DeployDSC.s.sol";
import {DSCEngine} from "../../src/DSCEngine.sol";
import {DecentralizedStableCoin} from "../../src/DecentralizedStableCoin.sol";
import {HelperConfig} from "../../script/HelperConfig.s.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/ERC20Mock.sol";
import {Test, console} from "forge-std/Test.sol";
import {MockV3Aggregator} from "../mocks/MockV3Aggregator.sol";
contract DSCELiquidateTest is Test {
DSCEngine public dsce;
DecentralizedStableCoin public dsc;
HelperConfig public helperConfig;
address public ethUsdPriceFeed;
address public btcUsdPriceFeed;
address public weth;
address public wbtc;
uint256 public deployerKey;
uint256 amountCollateral = 10 ether;
uint256 amountToMint = 100 ether;
address public user = address(1);
uint256 public constant STARTING_USER_BALANCE = 10 ether;
uint256 public constant MIN_HEALTH_FACTOR = 1e18;
uint256 public constant LIQUIDATION_THRESHOLD = 50;
// Liquidation
address public liquidator = makeAddr("liquidator");
address public attacker = makeAddr("attacker");
uint256 public collateralToCover = 20 ether;
function setUp() public {
DeployDSC deployer = new DeployDSC();
(dsc, dsce, helperConfig) = deployer.run();
(ethUsdPriceFeed, btcUsdPriceFeed, weth, wbtc, deployerKey) = helperConfig.activeNetworkConfig();
if (block.chainid == 31337) {
vm.deal(user, STARTING_USER_BALANCE);
}
ERC20Mock(weth).mint(user, STARTING_USER_BALANCE);
ERC20Mock(weth).mint(liquidator, STARTING_USER_BALANCE);
ERC20Mock(weth).mint(attacker, STARTING_USER_BALANCE);
}
function testLiquidate() public {
//User deposits collaterall and mints DSC(HealthFactor = 1);
vm.startPrank(user);
ERC20Mock(weth).approve(address(dsce), 1 ether);
dsce.depositCollateralAndMintDsc(weth, 1 ether, 1000e18);
vm.stopPrank();
//Case 1 When price goes down
console.log("price of ETH dropped from $2000 to $1800");
MockV3Aggregator(ethUsdPriceFeed).updateAnswer(180000000000);
//And user falls below minimum health factor
uint256 userHealthFactor = dsce.getHealthFactor(user);
console.log("User healthFactor after price drop:");
console.logUint(userHealthFactor);
console.log("User healthFactor converted 0.9");
console.log("---------------------");
//Liquidator see liqidation opportunity and mints just enough to make profit
//deposits collateral and mints DSC (healthFactor = 1)
vm.startPrank(liquidator);
ERC20Mock(weth).approve(address(dsce), 0.4 ether);
dsce.depositCollateralAndMintDsc(weth, 0.4 ether, 200e18);
vm.stopPrank();
(uint256 dscAmountBeforeLiquidate , ) = dsce.getAccountInformation(liquidator);
console.log("Liquidator DSC amount before liquidate");
console.logUint(dscAmountBeforeLiquidate);
console.log("Liquidator DSC amount converted 200");
console.log("-------------------------");
//liquidator liquidates user
console.log("==============================================");
console.log("Executing liquidate by liquidator");
vm.startPrank(liquidator);
dsc.approve(address(dsce), 200e18);
dsce.liquidate(weth, user, 200e18);
console.log("---------------------------");
//Liquidator amount of DSC minted after liquidate(Unchanged)
(uint256 dscAmountAfterLiquidate , ) = dsce.getAccountInformation(liquidator);
console.log("Liquidator DSC amount after liquidate");
console.logUint(dscAmountAfterLiquidate);
console.log("Liquidator DSC amount after liquidate converted 200");
console.log("--------------------------");
//Actual DSC amount liquidator has
uint256 bal= dsc.balanceOf(liquidator);
console.logUint(bal);
console.log("liquidator actual DSC balance after liquidate = 0");
vm.stopPrank();
uint256 liquidatorHealthFactorAfterLiquidate = dsce.getHealthFactor(liquidator);
console.logUint(liquidatorHealthFactorAfterLiquidate);
console.log("liquidator health factor converted = 1.8");
//Price drops from $1800 to $900
//liquidator health factor falls below minimum
MockV3Aggregator(ethUsdPriceFeed).updateAnswer(90000000000);
//Attacker can liquidate liquidator though liquidator, who holds no DSC and should be free of liquidation risks
vm.startPrank(attacker);
ERC20Mock(weth).approve(address(dsce), 2 ether);
dsc.approve(address(dsce), 100e18);
dsce.depositCollateralAndMintDsc(weth, 1 ether, 100e18);
dsce.liquidate(weth, liquidator, 100e18);
vm.stopPrank();
}
function testRedeemAfterLiquidation() public {
vm.startPrank(user);
ERC20Mock(weth).approve(address(dsce), 1 ether);
dsce.depositCollateralAndMintDsc(weth, 1 ether, 1000e18);
vm.stopPrank();
console.log("price of ETH dropped from $2000 to $1800");
MockV3Aggregator(ethUsdPriceFeed).updateAnswer(180000000000);
//And user falls below minimum health factor
uint256 userHealthFactor = dsce.getHealthFactor(user);
console.log("User healthFactor after price drop");
console.logUint(userHealthFactor);
console.log("User health converted with 1e18 = 0.9");
//Liquidator see liquidation opportunity and mints just enough to make profit
//deposits collateral and mints DSC (healthFactor = 1)
vm.startPrank(liquidator);
ERC20Mock(weth).approve(address(dsce), 0.4 ether);
dsce.depositCollateralAndMintDsc(weth, 0.4 ether, 200e18);
vm.stopPrank();
uint256 liquidateHealthFactor = dsce.getHealthFactor(liquidator);
console.logUint(liquidateHealthFactor);
//Amount of DSC minted before calling liquidate
(uint256 dscMinted, ) = dsce.getAccountInformation(liquidator);
console.log("Liquidator DSC minted before calling liquidate");
console.logUint(dscMinted);
console.log("Liquidator amount of DSC minted converted = 1000");
console.log("==============================================");
console.log("Executing liquidate by liquidator");
vm.startPrank(liquidator);
dsc.approve(address(dsce), 200e18);
dsce.liquidate(weth, user, 200e18);
//Liquidator tries to redeem collateral => will revert
vm.expectRevert();
dsce.redeemCollateral(weth, 0.4 ether);
vm.expectRevert();
dsce.redeemCollateralForDsc(weth, 0.4 ether, 200e18);
vm.stopPrank();
}
}
Log1:
price of ETH dropped from $2000 to $1800
User healthFactor after price drop:
900000000000000000
User healthFactor converted 0.9
---------------------
Liquidator DSC amount before liquidate
200000000000000000000
Liquidator DSC amount converted 200
-------------------------
==============================================
Executing liquidate by liquidator
---------------------------
Liquidator DSC amount after liquidate
200000000000000000000
Liquidator DSC amount after liquidate converted 200
--------------------------
0
liquidator actual DSC balance after liquidate = 0
1800000000000000000
liquidator health factor converted = 1.8
Log2:
price of ETH dropped from $2000 to $1800
User healthFactor after price drop
900000000000000000
User health converted with 1e18 = 0.9
1800000000000000000
Liquidator DSC minted before calling liquidate
200000000000000000000
Liquidator amount of DSC minted converted = 1000
==============================================
Executing liquidate by liquidator
liquidator unable to redeem full amount
Tools Used
Foundry
Recommendations
function liquidate(address collateral, address user, uint256 debtToCover)
external
moreThanZero(debtToCover)
nonReentrant
{
// need to check health factor of the user
uint256 startingUserHealthFactor = _healthFactor(user);
if (startingUserHealthFactor >= MIN_HEALTH_FACTOR) {
revert DSCEngine__HealthFactorOk();
}
uint256 tokenAmountFromDebtCovered = getTokenAmountFromUsd(collateral, debtToCover);
uint256 bonusCollateral = (tokenAmountFromDebtCovered * LIQUIDATION_BONUS) / LIQUIDATION_PRECISION;
uint256 totalCollateralToRedeem = tokenAmountFromDebtCovered + bonusCollateral;
_redeemCollateral(user, msg.sender, collateral, totalCollateralToRedeem);
+ s_DSCMinted[msg.sender] -= debtToCover;
_burnDsc(debtToCover, user, msg.sender);
uint256 endingUserHealthFactor = _healthFactor(user);
if (endingUserHealthFactor <= startingUserHealthFactor) {
revert DSCEngine__HealthFactorNotImproved();
}
_revertIfHealthFactorIsBroken(msg.sender);
}
Prize pool breakdown
Total prize
15,000 USDC
nSLOC
23664 USDC / LOC
14,000 USDC
1,000 USDC
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.