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

nfmelendez - Liquidation fee is wrongly charged to the liquidators #334

Closed
sherlock-admin2 opened this issue Aug 24, 2024 · 0 comments
Closed
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

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Aug 24, 2024

nfmelendez

High

Liquidation fee is wrongly charged to the liquidators

Summary

Liquidation fees should be charged to the position but instead is charged to the liquidator leading to less incentives to keep the system healthy

Vulnerability Detail

The PositionManager use the RiskModule to validate the liquidator attemp to liquidate a position. The LIQUIDATION_DISCOUNT in RiskModule::_validateSeizedAssetValue is used to increase the maxSeizedAssetValue cap so the liquidator can seize more assets so in other words the LIQUIDATION_DISCOUNT is the bonus the liquidator get for keep the system healthy. However the bonus that generates the incentive to liquidator to keep the system healthy is reduced because Sentiment is charging the liquidation fee in the PositionManager::_transferAssetsToLiquidator#280 to the liquidator's assetData[i].amt but should really charge the liquidation fee reducing the assets of the position because is the one that created the problem in the first place.

Here we can see that is taking the assets from the liquidator to the protocol:

    // @audit is charging fees to the amount of assets that the liquidator will seize.
    uint256 fee = liquidationFee.mulDiv(assetData[i].amt, 1e18);
    // @audit Protocol is getting profit from the liquidator instead of the owner of the bad debt which is the one who generated the bad debt. 
    Position(payable(position)).transfer(owner(), assetData[i].asset, fee);

Impact

  • Liquidators have less incentives because are paying with their bonus the liquidation fee.
  • The Position has incentives to be risky because they don't pay the liquidation fee with its assets.
  • SentimentV2 protocol profit from the liquidators instead of the unhealthy debt that position created in first place.

Code Snippet

https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/main/protocol-v2/src/RiskModule.sol#L155-L159

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

Tool used

  • Manual Review
  • VS code

Recommendation

One solution could be to increase maxSeizedAssetValue cap by the liquidation fee and the liquidation discount, so the liquidator collects the liquidation fee from the position and then Sentiment collects
the liquidation fee from the liquidator:

        uint256 maxSeizedAssetValue = debtRepaidValue.mulDiv(1e18, (1e18 - discount))
        + debtRepaidValue.mulDiv(1e18, (1e18 - liquidationFee)); // added by auditor, incresing the cap let the liquidator collect the liquidation fee that then willl be collected by the protocol.
        if (assetSeizedValue > maxSeizedAssetValue) {
            revert RiskModule_SeizedTooMuch(assetSeizedValue, maxSeizedAssetValue);
        }

Duplicate of #91

@github-actions github-actions bot closed this as completed Sep 5, 2024
@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. labels Sep 5, 2024
@sherlock-admin4 sherlock-admin4 changed the title Able Burgundy Kookaburra - Liquidation fee is wrongly charged to the liquidators nfmelendez - Liquidation fee is wrongly charged to the liquidators 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
Projects
None yet
Development

No branches or pull requests

2 participants