Fujicracy / fuji-v2

Cross-chain money market aggregator
https://fuji-v2-frontend.vercel.app
15 stars 10 forks source link

BeforeTokenTransfer affects partial liquidations #401

Closed 0xdcota closed 1 year ago

0xdcota commented 1 year ago

[H-4] Partial Liquidations may not be possible in some cases due to check in beforeTokenTransfer

Description

In certain cases, partial liquidations may not be possible, and the vault will remain vulnerable until the health factor drops below FULL_LIQUIDATION_THRESHOLD. For example, consider the following scenario:

1. 
Alice's collateral is worth $4000, and she has borrowed 2 assets of $1500 each,
With a maximum loan-to-value ratio of 75% and liquidation ratio of 80%.

Collateral: $4000
Debt: 2 * ($1500)  

2. 
Debt asset's price increases to $1650
Collateral: $4000
Debt: 2 ($1650)
The health factor: 4000 * 0.80/ 3300 = 0.96969697 > 0.95
Vault intends to liquidate half of the debt position.

The assets liquidator would get
1 (half debt) * 1650 / 0.9 (liq discount) = 1833.333333333
Alice still has a debt of 1 * 1650, 
Which by 0.75 LTV, utilizes 2200 of Alice's collateral.

maxRedeem for Alice = 4000 - 2200 = 1800
What Alice need to pay to the liquidator = 1833.33

1833 > 1800
Code Reverts !! On this [line](https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/abstracts/BaseVault.sol#L594)

BaseVault.sol#L594
function _beforeTokenTransfer(address from, address to, uint256 amount) internal view override {
    to; 
    if (from != address(0)) {
    #L594 require(amount <= maxRedeem(from), "Transfer more than max");
    }
}

Remediation to consider

Consider adding a pass for this case inside _beforeTokenTransfer. Please note that it cannot be skipped for all burns, as the withdraw relies on the burning of tokens to determine the amount of collateral a user can withdraw.