Skip to content
This repository has been archived by the owner on Sep 3, 2023. It is now read-only.

peanuts - liquidate ()should handle the case if the repaid amount is greater than the loan amount #265

Closed
github-actions bot opened this issue Mar 10, 2023 · 0 comments
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout

Comments

@github-actions
Copy link

peanuts

medium

liquidate ()should handle the case if the repaid amount is greater than the loan amount

Summary

liquidate() should handle the case if the repaid amount is greater than the loan amount.

Vulnerability Detail

When liquidating loans in Surge, the liquidator can call liquidate() and pay the borrowers debt partially or fully in exchange for collateral. If liquidator intends to repay the debt in full, then he should set the amount to type(uint).max. If not, the borrower will set an arbitrary amount. However, the liquidator does not know the debt of the borrower until getDebtOf() is called. There may be cases when the amount he intends to pay back is greater than the loan amount. If that is the case, the borrower should also be able to repay in full.

    function liquidate(address borrower, uint amount) external {
        uint _loanTokenBalance = LOAN_TOKEN.balanceOf(address(this));
        (address _feeRecipient, uint _feeMantissa) = FACTORY.getFee();
        (  
            uint _currentTotalSupply,
            uint _accruedFeeShares,
            uint _currentCollateralRatioMantissa,
            uint _currentTotalDebt
        ) = getCurrentState(
            _loanTokenBalance,
            _feeMantissa,
            lastCollateralRatioMantissa,
            totalSupply,
            lastAccrueInterestTime,
            lastTotalDebt
        );


        uint collateralBalance = collateralBalanceOf[borrower];
        uint _debtSharesSupply = debtSharesSupply;
        uint userDebt = getDebtOf(debtSharesBalanceOf[borrower], _debtSharesSupply, _currentTotalDebt);
        uint userCollateralRatioMantissa = userDebt * 1e18 / collateralBalance;
        require(userCollateralRatioMantissa > _currentCollateralRatioMantissa, "Pool: borrower not liquidatable");


        address _borrower = borrower; // avoid stack too deep
        uint _amount = amount; // avoid stack too deep
        uint _shares;
        uint collateralReward;
@--audit only checks type(uint).max and _amount == userDebt but should also check _amount > userDebt
        if(_amount == type(uint).max || _amount == userDebt) {
            collateralReward = collateralBalance;
            _shares = debtSharesBalanceOf[_borrower];
            _amount = userDebt;
        } else {
            uint userInvertedCollateralRatioMantissa = collateralBalance * 1e18 / userDebt;
            collateralReward = _amount * userInvertedCollateralRatioMantissa / 1e18; // rounds down
            _shares = tokenToShares(_amount, _currentTotalDebt, _debtSharesSupply, false);
        }
        _currentTotalDebt -= _amount;

If amount > userDebt, then the else part will execute and the transaction will revert because of underflow.

A similar issue can be seen here: sherlock-audit/2023-01-cooler-judging#327

Impact

liquidation will underflow if _amount is greater than userDebt.

Code Snippet

https://github.com/sherlock-audit/2023-02-surge/blob/main/surge-protocol-v1/src/Pool.sol#L504-L530

Tool used

Manual Review

Recommendation

Recommend updating the logic to take care of cases where _amount > userDebt

-        if(_amount == type(uint).max || _amount == userDebt) {
+        if(_amount == type(uint).max || _amount >= userDebt) {
            collateralReward = collateralBalance;
            _shares = debtSharesBalanceOf[_borrower];
            _amount = userDebt;
@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label Mar 10, 2023
@sherlock-admin sherlock-admin added the Non-Reward This issue will not receive a payout label Mar 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

1 participant