[BOS-01M] Improper Handling of Refund

[0xfornax]

BOS-01M: Improper Handling of Refund

Type	Severity	Location
Logical Fault		BorrowerOperations.sol:L313, L314

Description:

The potential refund a user would acquire from the FeeCollector as part of their vessel closure is factored in the repayment amount in the latest implementation of BorrowerOperations.

A caveat of this adjustment is that if the refund received as part of a vessel's closure exceeds the debt the vessel has, the vessel would not be possible to close.

Impact:

While a user will still be able to close their vessel by increasing its debt momentarily and invoking BorrowerOperations::closeVessel, the outlined behaviour is incorrect and can cause significant user distress.

Example:

function closeVessel(address _asset) external override {
    _requireVesselIsActive(_asset, msg.sender);
    uint256 price = IPriceFeed(priceFeed).fetchPrice(_asset);
    _requireNotInRecoveryMode(_asset, price);

    IVesselManager(vesselManager).applyPendingRewards(_asset, msg.sender);

    uint256 coll = IVesselManager(vesselManager).getVesselColl(_asset, msg.sender);
    uint256 debt = IVesselManager(vesselManager).getVesselDebt(_asset, msg.sender);

    uint256 gasCompensation = IAdminContract(adminContract).getDebtTokenGasCompensation(_asset);
    uint256 refund = IFeeCollector(feeCollector).simulateRefund(msg.sender, _asset, 1 ether);
    uint256 netDebt = debt - gasCompensation - refund;

    _requireSufficientDebtTokenBalance(msg.sender, netDebt);

    uint256 newTCR = _getNewTCRFromVesselChange(_asset, coll, false, debt, false, price);
    _requireNewTCRisAboveCCR(_asset, newTCR);

    IVesselManager(vesselManager).removeStake(_asset, msg.sender);
    IVesselManager(vesselManager).closeVessel(_asset, msg.sender);

    emit VesselUpdated(_asset, msg.sender, 0, 0, 0, BorrowerOperation.closeVessel);

    // Burn the repaid debt tokens from the user's balance and the gas compensation from the Gas Pool
    _repayDebtTokens(_asset, msg.sender, netDebt, refund);
    if (gasCompensation != 0) {
        _repayDebtTokens(_asset, gasPoolAddress, gasCompensation, 0);
    }

    // Signal to the fee collector that debt has been paid in full
    IFeeCollector(feeCollector).closeDebt(msg.sender, _asset);

    // Send the collateral back to the user
    IActivePool(activePool).sendAsset(_asset, msg.sender, coll);
}

Recommendation:

We advise the code to properly handle such a case and to introduce a new argument to the FeeCollector::closeDebt call that signifies what value (if any) should be transmitted to the user as part of a vessel's closure given that the debt of the vessel may be less than the refund acquired from the FeeCollector.

[spider-g]

Thank you for your report. I would like to clarify that the debt will never be less than the refund. This is because whenever the debt decreases, such as through a partial payoff, the refund will also be adjusted proportionally downwards. This adjustment is carried out through the functions VesselManager.decreaseVesselDebt and FeeCollector.decreaseDebt.