ast3ros - [H-01] Liquidation reward can be wrongly calculated because of round.

[github-actions]

ast3ros

high

# [H-01] Liquidation reward can be wrongly calculated because of round.

Summary

Due to rounding errors in the collateralReward variable, the liquidation reward may not be calculated accurately.

Vulnerability Detail

When liquidation a loan, a user will call the function pool.liquidate.

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

The formula for calculating the collateral reward amount for a partial liquidation is:

        uint userInvertedCollateralRatioMantissa = collateralBalance * 1e18 / userDebt;
        collateralReward = _amount * userInvertedCollateralRatioMantissa / 1e18; // rounds down

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

The error occurs when _amount * userInvertedCollateralRatioMantissa produces a number lower than 1e18. This causes the collateralReward to be rounded down to 0.

This scenario is common when the Collateral Ratio is high (For example in case of borrow USDT - collateral ETH, the collateral ratio could be 2000e18 or 3000e18).
This results in a low userInvertedCollateralRatioMantissa (around e15). Therefore, when the liquidation amount is less than e3, the _amount * userInvertedCollateralRatioMantissa will be less than e18.
And _amount * userInvertedCollateralRatioMantissa / 1e18 is rounded down to 0.

POC: please put in ./test/Pool.t.sol

    function testRewardError() external {
        uint amount = 1e18;
        MockERC20 collateralToken = new MockERC20(amount*10000, 18);
        MockERC20 loanToken = new MockERC20(amount * 10000, 18);
        // Set collateral max to 5000e18
        Pool pool = factory.deploySurgePool(IERC20(address(collateralToken)), IERC20(address(loanToken)), 5000e18, 0.5e18, 1e15, 1e15, 0.1e18, 0.4e18, 0.6e18);
        loanToken.approve(address(pool), type(uint).max);
        collateralToken.approve(address(pool), type(uint).max);

        // Deposit all 10000e18
        pool.deposit(amount * 10000);

        // Colateral: 1e18
        pool.addCollateral(address(this), amount);

        // Borrow: 5000e18
        pool.borrow(amount * 5000);
        vm.warp(block.timestamp + 365 days);

        // Accrue interest
        pool.withdraw(0);

        // Liquidate 1000 unit
        uint collateralTokenBefore = collateralToken.balanceOf(address(this));
        pool.liquidate(address(this), 1000);
        uint collateralTokenAfter = collateralToken.balanceOf(address(this));

        // No collateral reward is received
        assertEq(collateralTokenBefore, collateralTokenAfter);
    }

Impact

When a user attempts to partially liquidate a small amount of debt, they may not receive any collateral reward due to rounding errors in the calculation, even though they have paid for the debt. This risk increases when the ERC20 token has fewer decimals and the collateral ratio is higher.

This bug could discourage users from liquidating debt and disrupt the pool's operation.

Code Snippet

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

Tool used

Manual Review

Recommendation

• Instead of dividing by e^18, calculate the reward dynamically based on the decimals of the tokens.
• Verify that the calculation does not round down to 0.

Duplicate of #122

[thangtranth]

Escalate for 10 USDC

As in my detailed analysis and POC, it is clearly that the liquidation reward may not be calculated accurately because of fixing the denominator to 1e18.
In the POC, liquidation reward is round down to 0 and the liquidator get nothing (No collateral reward is received).

[sherlock-admin]

Escalate for 10 USDC

As in my detailed analysis and POC, it is clearly that the liquidation reward may not be calculated accurately because of fixing the denominator to 1e18.
In the POC, liquidation reward is round down to 0 and the liquidator get nothing (No collateral reward is received).

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[hrishibhat]

Escalation accepted

This is a valid duplicate of #122

[sherlock-admin]

Escalation accepted

This is a valid duplicate of #122

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.