Incorrect calculation of newCumulativeIndex in function calcDecrease

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-07-loopfi/blob/main/src/CDPVault.sol#L652

Vulnerability details

In the contract CDPVault.sol, the function calcDecrease calculates newCumulativeIndex in line 703 when amountToRepay < interestAccrued with profit which is:

https://github.com/code-423n4/2024-07-loopfi/blob/main/src/CDPVault.sol#L703

                    newCumulativeIndex =
                    (INDEX_PRECISION * cumulativeIndexNow * cumulativeIndexLastUpdate) /
                    (INDEX_PRECISION *
                        cumulativeIndexNow -
                        (INDEX_PRECISION * profit * cumulativeIndexLastUpdate) /
                        debt); // U:[CL-3]

However, the profit constains the cumulativeQuotaInterest, so it can not be used to calculate newCumulativeIndex.

https://github.com/code-423n4/2024-07-loopfi/blob/main/src/CDPVault.sol#L668

            if (amountToRepay >= cumulativeQuotaInterest) {
                amountToRepay -= cumulativeQuotaInterest; // U:[CL-3]
                profit += cumulativeQuotaInterest; // U:[CL-3]

                newCumulativeQuotaInterest = 0; // U:[CL-3]
            }

For example, when the left amountToRepay can cover the interestAccrued, the newCumulativeIndex should be cumulativeIndexNow as in line 692:

Because : interestAccrued == (debt * cumulativeIndexNow) / cumulativeIndexLastUpdate - debt

https://github.com/code-423n4/2024-07-loopfi/blob/main/src/CDPVault.sol#L692

            if (amountToRepay >= interestAccrued) {
                amountToRepay -= interestAccrued;

                profit += interestAccrued;

                newCumulativeIndex = cumulativeIndexNow;
            }

However, when the left amountToRepay = interestAccrued - 1, and profit will be cumulativeQuotaInterest+interestAccrued - 1. And the calculation of newCumulativeIndex will be larger than cumulativeIndexNow because the cumulativeQuotaInterest+interestAccrued - 1 will be larger than interestAccrued which is totaly wrong, since it exceeds the limit
cumulativeIndexNow.

https://github.com/code-423n4/2024-07-loopfi/blob/main/src/CDPVault.sol#L703

                    newCumulativeIndex =
                    (INDEX_PRECISION * cumulativeIndexNow * cumulativeIndexLastUpdate) /
                    (INDEX_PRECISION *
                        cumulativeIndexNow -
                        (INDEX_PRECISION * profit * cumulativeIndexLastUpdate) /
                        debt); // U:[CL-3]

Impact

Position.cumulativeIndexLastUpdate will be updated with incorrect newCumulativeIndex.
And less interest accrued will be charged from users. And the position which should be liquidated will not be liquidated due the the wrong Position.cumulativeIndexLastUpdate.

Proof of Concept

Paste this test in CDPVault.t.sol:

    function test_calcDecrese_poc() public {
        //CDPVault vault = createCDPVault(token, 150 ether, 0, 1.25 ether, 1.0 ether, 0);
        CDPVault vault = createCDPVault(token, 150 ether, 0, 1.25 ether, 1 ether, 0.95 ether);
        createGaugeAndSetGauge(address(vault));

        // create position
        token.mint(address(this), 1000 ether);
        token.approve(address(vault), 100 ether);
        vault.modifyCollateralAndDebt(address(this), address(this), address(this), 100 ether, 70 ether);

        vm.warp(block.timestamp + 865 days);

     

        (, uint256 totalInterest, ) = vault.getDebtInfo(address(this));

        mockWETH.mint(address(this), 500 ether);
        mockWETH.approve(address(vault), 200 ether);
        
        int256 repayAmount = -(int256(totalInterest - 1));

        vault.modifyCollateralAndDebt(address(this), address(this), address(this), 0, repayAmount);
        
        (,,,uint256 cumulativeIndexLastUpdate,, ) = vault.positions(address(this));
        uint256 cumulativeIndexNow = liquidityPool.baseInterestIndex();

        console.log("cumulativeIndexLastUpdate",cumulativeIndexLastUpdate);
        console.log("cumulativeIndexNow",cumulativeIndexNow);


    }

The poc result will be :

cumulativeIndexLastUpdate 1242341891861284810269427100

cumulativeIndexNow 1239397711761394368218295007

cumulativeIndexLastUpdate>cumulativeIndexNow

which is wrong

Tool used

Manual Review

Recommended Mitigation Steps

Use the code as below:

            else {
                // If amount is not enough to repay interest, then send all to the stakers and update index
                profit += amountToRepay; // U:[CL-3]
                

                newCumulativeIndex =
                    (INDEX_PRECISION * cumulativeIndexNow * cumulativeIndexLastUpdate) /
                    (INDEX_PRECISION *
                        cumulativeIndexNow -
                        (INDEX_PRECISION * amountToRepay * cumulativeIndexLastUpdate) /
                        debt); // U:[CL-3]

                amountToRepay = 0; // U:[CL-3]
            }

Assessed type

Other

[0xtj24]

Fix in LoopFi/loop-contracts#78

[c4-judge]

koolexcrypto marked the issue as satisfactory

[c4-judge]

koolexcrypto marked the issue as selected for report