search

sherlock-audit / 2023-12-arcadia-judging

19 stars 15 forks source link

FCSE507 - The current interest is not included in the used margin calculation. #155

Closed sherlock-admin2 closed 9 months ago

sherlock-admin2 commented 9 months ago

FCSE507

medium

The current interest is not included in the used margin calculation.

Summary

We process interests before executing some actions in the Lending Pool to apply unrealized debt. However, the used margin calculation doesn't consider the unrealized debt. As a result, an account that can actually be liquidated may be marked as healthy.

Vulnerability Detail

When an account is liquidatable, someone initiates liquidation. This imposes additional debt on the account, including initiation rewards, termination rewards, etc.

function startLiquidation(address initiator, uint256 minimumMargin_) {
    (uint256 initiationReward, uint256 terminationReward, uint256 liquidationPenalty) =
        _calculateRewards(startDebt, minimumMargin_);

    // Mint the liquidation incentives as extra debt towards the Account.
    _deposit(initiationReward + liquidationPenalty + terminationReward, msg.sender);
}

After some time, the collateral value increases due to market conditions.

Anyone can settle the auction for this account.

function _settleAuction(address account, AuctionInformation storage auctionInformation_) {
    uint256 collateralValue = IAccount(account).getCollateralValue();
    uint256 usedMargin = IAccount(account).getUsedMargin();

    if (collateralValue >= usedMargin || usedMargin == minimumMargin) {
        ILendingPool(creditor).settleLiquidationHappyFlow(account, startDebt, minimumMargin, msg.sender);
    }
}

Here, getUsedMargin() doesn't consider unrealized debt, so the collateral value may fall between the getUsedMargin() value and the actual used margin. This implies that the auction of this account can be concluded, and someone can initiate liquidation for this account again immediately.

Impact

This incurs additional debt to the accounts and is unfair.

Code Snippet

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/lending-v2/src/LendingPool.sol#L876-L877 https://github.com/sherlock-audit/2023-12-arcadia/blob/main/lending-v2/src/Liquidator.sol#L444-L445

Tool used

Manual Review

Recommendation

The getUsedMargin calculation should also include unrealized debt.

Duplicate of #1

sherlock-admin2 commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid

nevillehuang commented 9 months ago

Invalid, agree with sponsors comments:

UnrealisedDebt is taken into account, the flow is:

  • account.getUsedMargin → lendingPool.getOpenPosition → ERC4626.maxWithdraw → ERC4626.convertToAssets → lendingPool.totalAssets, the latter taking into account the unrealised debt.
  • Confusion may exist as to know which function is overwritten