sherlock-audit / 2023-02-surge-judging

4 stars 1 forks source link

y1cunhui - Interest Accrual is Far More Fast than Expected #289

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

y1cunhui

high

Interest Accrual is Far More Fast than Expected

Summary

With more operation executed in the pool, the interest accrual will become far much more than expected.

Vulnerability Detail

Notice that, the immutable variables MIN_RATE, SURGE_RATE, MAX_RATE are supposed to be the APR, i.e, accrued ANNUALLY. However, the interest is actually accrued EACH TIME the getCurrentState is called.

        // 8. Calculate the borrow rate
        uint _borrowRate = getBorrowRateMantissa(_util, SURGE_MANTISSA, MIN_RATE, SURGE_RATE, MAX_RATE);
        // 9. Calculate the interest
        uint _interest = _totalDebt * _borrowRate * _timeDelta / (365 days * 1e18); // does the optimizer optimize this? or should it be a constant?
        // 10. Update the total debt
        _currentTotalDebt += _interest;

This will make the interest accrued far more fast than it should be. As shown in the PoC below: (Full PoC source code here, in case you want to run it.) (To run this PoC yourself, you may need to change the visibility of some functions in Pool.sol)

function testInterestAccrualOnce() external {
        MockERC20 collateralToken = new MockERC20(10000e18, 18);
        MockERC20 loanToken = new MockERC20(10000e18, 18);
        Pool pool = factory.deploySurgePool(IERC20(address(collateralToken)), IERC20(address(loanToken)), 1e18, 0.8e18, 1e15, 1e15, 10e18, 10e18, 10e18);
        loanToken.approve(address(pool), type(uint).max);
        collateralToken.approve(address(pool), type(uint).max);
        pool.deposit(100e18);
        pool.addCollateral(address(this), 20e18);
        pool.borrow(20e18);
        vm.warp(block.timestamp + 365 days);
        uint _loanTokenBalance = IERC20(address(loanToken)).balanceOf(address(pool));
        (address _feeRecipient, uint _feeMantissa) = factory.getFee();
        (  ,,,
            uint _currentTotalDebt
        ) = pool.getCurrentState(
            _loanTokenBalance,
            _feeMantissa,
            pool.lastCollateralRatioMantissa(),
            pool.totalSupply(),
            pool.lastAccrueInterestTime(),
            pool.lastTotalDebt()
        );
        console.log("Total Debt after 1 year, just 1 operation:\n %d \n", _currentTotalDebt);
        //2 
        Pool pool2 = factory.deploySurgePool(IERC20(address(collateralToken)), IERC20(address(loanToken)), 1e18, 0.8e18, 1e15, 1e15, 10e18, 10e18, 10e18);
       ...// similar things as pool1
        for (uint i = 0; i < 2; i++ ) {
            vm.warp(block.timestamp + 180 days);
            pool2.deposit(1e18);
            pool2.withdraw(1e18);
        }
        ...// similar as pool1
        console.log("Total Debt after 1 year, 2 operations:\n %d \n", _currentTotalDebt2);
        //3
        Pool pool3 = factory.deploySurgePool(IERC20(address(collateralToken)), IERC20(address(loanToken)), 1e18, 0.8e18, 1e15, 1e15, 10e18, 10e18, 10e18);
        ...// similar as pool1
        for (uint i = 0; i < 12; i++ ) {
            vm.warp(block.timestamp + 30 days);
            pool3.deposit(1e18);
            pool3.withdraw(1e18);
        }
         ...// similar as pool1
        console.log("Total Debt after 1 year, 1 operation per month:\n %d \n", _currentTotalDebt3);
    }

The result is

[PASS] testInterestAccrualOnce() (gas: 8123416)
Logs:
  Total Debt after 1 year, just 1 operation:
 220000000000000000000

  Total Debt after 1 year, 2 operations:
 703655470069431413023

  Total Debt after 1 year, 1 operation per month:
 7643819010480261788044

We keep the same settings in the three groups, and always deposit 20e18, the interest rate is set to be 1,000% APR(although unreasonably high, just to show the impact) everywhere. It is supposed that the total debt should be 220e18 after 1 year. However, this is just true for no operation between the borrow and repay. Just 2 operations will make the actual APR become ~3,500%. And if there is 1 operation per month(attention: FOR THE WHOLE POOL, not only this operator), the actual APR becomes ~38,000%. Considering the MEV bots, there may be tens/hunderds of operations in the pool EVERYDAY, the actual interest rate will become enormous beyond imagination.

Impact

This will make the actual borrow rate FAR MORE than the expected rate.

Code Snippet

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

Tool used

Manual Review

Foundry

Recommendation

Consider designing the interest calculation method from scratch.

Duplicate of #55