Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ravikiran.web3 - validation to liquidation should include Liquidation fees as they are liable from customers #100

Closed
sherlock-admin4 opened this issue Aug 24, 2024 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Aug 24, 2024

ravikiran.web3

High

validation to liquidation should include Liquidation fees as they are liable from customers

Summary

When validation for liquidation is computed in the risk module, it computes the value for debt and collateral including the interest accrued, but the valuation does not include the liquidation fees that needs to be born by the customer. Hence liquidation fees should be accounted in the validation logic.

The liquidation fee is charged by the liquidator to liquidate positions that are below the collateral factor. If the valuation falls to the extent that liquidation fees cannot be covered, then liquidation may not go through in time leaving the position as bad debt.

Root Cause

https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/main/protocol-v2/src/PositionManager.sol#L430-L444

PositionManager::liquidate(...) function checks with the risk Engine, if the position is validate for Liquidation. In the below code snippet, riskEngine.validateLiquidation(...) does not take into account the liquidation fee charged by the liquidator which needs to be born by the customer. Hence, the liable amount from the customer should include the borrowed amount + interest accrued + liquidation fees. Since the riskEngine.validateLiquidation(...) does not account for liquidation fees, if the valuation falls to the extent where liquidation fees cannot be covered, the liquidation process will revert.

    function liquidate(
        address position,
        DebtData[] calldata debtData,
        AssetData[] calldata assetData
    ) external nonReentrant {
 ==>       riskEngine.validateLiquidation(position, debtData, assetData);

Internal pre-conditions

  1. Liquidation validation should check for the position by accounting for
    Borrowed amount
    Accrued Interest
    Liquidation fee

  2. The liquidation should happen before the collateral falls short of the some of above 3 items.

External pre-conditions

No response

Attack Path

No response

Impact

When the valuation of the collateral falls below the sum of borrowed funds + accrued interest + liquidation fees, then there is a risk that liquidation will not go through as there may not be enough funds left in the position after liquidation to cover.

In such a case, since liquidator will not have the necessary motivation to liquidation such positions leading to bad debts for the protocol.

PoC

No response

Mitigation

Account for liquidation fees in the riskEngine.validateLiquidation(...) so that when the assessment for liquidation of a position is done, it is done with all the fees due. This will ensure that positions are liquidated timely and liquidators are motivated to execute the liquidation in a timely way.

Duplicate of #91

@github-actions github-actions bot added the Medium A Medium severity issue. label Sep 5, 2024
@sherlock-admin3 sherlock-admin3 added Sponsor Confirmed The sponsor acknowledged this issue is valid Sponsor Disputed The sponsor disputed this issue's validity Will Fix The sponsor confirmed this issue will be fixed Won't Fix The sponsor confirmed this issue will not be fixed and removed Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Sep 9, 2024
@z3s z3s added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Sep 15, 2024
@z3s z3s closed this as completed Sep 15, 2024
@sherlock-admin4 sherlock-admin4 changed the title Fast Graphite Aardvark - validation to liquidation should include Liquidation fees as they are liable from customers ravikiran.web3 - validation to liquidation should include Liquidation fees as they are liable from customers Sep 15, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

3 participants