Vulnerability Details
The liquidation mechanism in the protocol miscalculates the liquidation bonus by subtracting the bonus collateral from the user’s deposited collateral without considering its effect on the user's health factor. This miscalculation results in a consistently reduced health factor for the user after liquidation, even when the intended goal is to improve it. Additionally, if the user's collateral balance is insufficient to cover both the debt and the liquidation bonus, the function reverts due to underflow, rendering the entire liquidation process non-functional.
Impact
The incorrect bonus calculation prevents successful liquidations, making it impossible for liquidators to operate effectively.
If the user's collateral balance is lower than token_amount_from_debt_covered + bonus_collateral
, the protocol reverts due to underflow.
Tools Used
Manual review
Proof of Concept
Protocol's own test is a proof of this - run the following test with a small change, and it will revert due to not improving health factor:
def test_must_improve_health_factor_on_liquidation(
some_user, liquidator, weth, eth_usd, wbtc, btc_usd
):
# Setup mock DSC
mock_dsc = mock_more_debt_dsc.deploy(eth_usd)
token_addresses = [weth, wbtc]
feed_addresses = [eth_usd, btc_usd]
dsce = dsc_engine.deploy(token_addresses, feed_addresses, mock_dsc)
mock_dsc.set_minter(dsce.address, True)
mock_dsc.transfer_ownership(dsce)
# Setup user position
with boa.env.prank(some_user):
weth.approve(dsce, COLLATERAL_AMOUNT)
dsce.deposit_collateral_and_mint_dsc(weth, COLLATERAL_AMOUNT, AMOUNT_TO_MINT)
# Setup liquidator
collateral_to_cover = to_wei(1, "ether")
weth.mint(liquidator, collateral_to_cover)
with boa.env.prank(liquidator):
weth.approve(dsce, collateral_to_cover)
debt_to_cover = to_wei(10, "ether")
dsce.deposit_collateral_and_mint_dsc(weth, collateral_to_cover, AMOUNT_TO_MINT)
mock_dsc.approve(dsce, debt_to_cover)
# Update price to trigger liquidation
eth_usd_updated_price = 18 * 10**8 # 1 ETH = $18
eth_usd.updateAnswer(eth_usd_updated_price)
- with boa.reverts("DSCEngine__HealthFactorNotImproved"):
dsce.liquidate(weth, some_user, debt_to_cover)
This test proves that underflow can also happen:
def test_liquidation_underflow(
some_user, liquidator, weth, eth_usd, dsc, dsce
):
with boa.env.prank(some_user):
weth.approve(dsce, COLLATERAL_AMOUNT)
dsce.deposit_collateral_and_mint_dsc(weth, COLLATERAL_AMOUNT, AMOUNT_TO_MINT)
weth.mint(liquidator, COLLATERAL_TO_COVER)
with boa.env.prank(liquidator):
weth.approve(dsce, COLLATERAL_TO_COVER)
dsce.deposit_collateral_and_mint_dsc(weth, COLLATERAL_TO_COVER, AMOUNT_TO_MINT)
eth_usd.updateAnswer(10 * 10**8)
debt_to_cover = AMOUNT_TO_MINT
with boa.env.prank(liquidator), boa.reverts("safesub"):
dsce.liquidate(weth, some_user, debt_to_cover)
Recommendations
Consider removing the bonus mechanism entirely. If bonus is a must, consider creating a governance-controlled reward system, and do not tie bonus to user's collateral.