wise-foundation / lending-audit

5 stars 4 forks source link

[FMR-01M] Inexistent Accommodation of Inexistent Incentives #110

Open vm06007 opened 1 year ago

vm06007 commented 1 year ago

FMR-01M: Inexistent Accommodation of Inexistent Incentives

Type Severity Location
Logical Fault FeeManager.sol:L574-L577, L586-L590

Description:

The FeeManager::payBackBadDebtForToken function does not account for a scenario whereby the Wise system has not acquired sufficient fees and thus cannot fulfil the payback incentive.

Impact:

The system will presently prevent payback operations entirely when it has not acquired sufficient fees to provide incentives to its users, a trait that can be exacerbated in extreme market conditions.

Example:

function payBackBadDebtForToken(
    uint256 _nftId,
    address _paybackToken,
    address _receivingToken,
    uint256 _shares
)
    external
    returns (
        uint256 paybackAmount,
        uint256 receivingAmount
    )
{
    updatePositionCurrentBadDebt(
        _nftId
    );

    if (badDebtPosition[_nftId] == 0) {
        return (
            0,
            0
        );
    }

    paybackAmount = WISE_LENDING.paybackAmount(
        _paybackToken,
        _shares
    );

    WISE_LENDING.corePaybackFeeManager(
        _paybackToken,
        _nftId,
        paybackAmount,
        _shares
    );

    _updateUserBadDebt(
        _nftId
    );

    receivingAmount = getReceivingToken(
        _paybackToken,
        _receivingToken,
        paybackAmount
    );

    _decreaseFeeTokens(
        _receivingToken,
        receivingAmount
    );

    _safeTransferFrom(
        _paybackToken,
        msg.sender,
        address(WISE_LENDING),
        paybackAmount
    );

    _safeTransfer(
        _receivingToken,
        msg.sender,
        receivingAmount
    );

    emit PayedBackBadDebt(
        _nftId,
        msg.sender,
        _paybackToken,
        _receivingToken,
        paybackAmount,
        block.timestamp
    );
}

Recommendation:

We advise the FeeManagerHelper::_decreaseFeeTokens as well as TransferHelper::_safeTransfer statements in the FeeManager::payBackBadDebtForToken function to be wrapped in an if conditional that ensures there are sufficient feeTokens in the system. Alternatively, we advise the receivingAmount to be set as the minimum between receivingAmount and feeTokens as well as for the function to execute the TransferHelper::_safeTransfer solely when the minimum is non-zero.

We consider either of the two proposed solutions as adequate in alleviating this exhibit.

vm06007 commented 1 year ago

Here WISE team aknowledged that auditor does not understand the system well:

Auditors suggestions are not necessary:

We believe auditor do not understand the payback functionality and the other functions too well. We believe that you think that once user accumulated badDebt in an NFT position user can only use payBackBadDebtForToken function but that is not the case and its just an additional extra function. There's many other options.

So auditors statement is wrong. And then if condition auditor suggests doesnt add anything sice it would fail in _decreaseFeeTokens anyway.

Also suggestion basically negates the fix percentage incentive and would need a complete rewrite with no additional benefit.

Team understands that maybe auditor thinks about capital efficiency is better this way, say if users also would do it for less. But this is solved by fees slowly accumulating anyway making it an effective auction.

Proposed improvement from auditor side does not make things better. Instead it even tricks users when letting the function pass with the minimal existing amount of fees inside fee manager.