Liquidations can be DOSd

Summary

Liquidation calls can be frontrun and reverted, since it requires a specific amount to be passed. This can be used toy extra time by denying complete liquidations continuously, essentially creating a DOS attack.

Vulnerability Detail

The function liquidate takes the parameter amount which is used to determine how much of the collateral needs to be liquidated. This calculation is done in the function _calculateLiquidateAmount which also has a require statement.

https://github.com/sherlock-audit/2023-04-jojo/blob/main/JUSDV1/src/Impl/JUSDBank.sol#L379-L438

require(
    amount <= liquidatedInfo.depositBalance[collateral],
    JUSDErrors.LIQUIDATE_AMOUNT_IS_TOO_BIG
);

This require statement ensures that the amount being liquidated is less than the amount of collateral that the user has deposited. If the passed amount is higher, the function call gets reverted. The person being liquidated can take advantage of this fact to deny liquidations. They can self-liquidate by as small an amount as 1 wei, to make sure that the amount passed in the function gets invalidated later, if the self-liquidation gets included in a block before the actual liquidation operation. Since front-running is common, and since normal liquidation calls are done with the entire holding amounts for maximum profits, this is considered a high severity issue.

The attack can be carried out via the following steps:

Alice has account with 1e18 ETH deposited. She is up for liquidation.
Bob calls liquidate and passes 1e18 as the amount, going for a complete liquidation.
Alice frontruns that transaction and calls liquidate with 1 as amount, either through their own account or through some dummy account. This reduces the total deposit amounts to 1e18-1 ETH.
Bob's liquidation attempt reverts, since the amount passed (1e18) is now larger than the deposit amount (1e18-1).
Alice prevent's their own liquidation, buying more time to have the prices fall enough that it becomes bad debt.

Impact

DOS for liquidations.

Code Snippet

https://github.com/sherlock-audit/2023-04-jojo/blob/main/JUSDV1/src/Impl/JUSDBank.sol#L379-L438

Tool used

Manual Review

Recommendation

If amount passed is larger than the deposited amount, set it to the max deposited amount, instead of reverting the transaction.

Duplicate of #221

JoscelynFarr commented 1 year ago

If users can prevent their account to be not liquidated, that's good

Trumpero commented 1 year ago

I believe this issue is a duplicate of #221

JoscelynFarr commented 1 year ago

fix link: https://github.com/JOJOexchange/JUSDV1/commit/1a2c7654fabab7d3ec45a7f3e590202ba6779e9a

IAm0x52 commented 1 year ago

Fix here will not work. Amount is adjusted inside _calculateLiquidateAmount but the unmodified value is passed into _afterLiquidateOperation. This attempts to _withdraw amount from liquidated but liquidated won't have enough collateral causing the call to revert.