052.md

Low Tangerine Cod

High

borrowers will not be able to withdraw their funds due to noOfBorrowers underflow in BorrowLib

Summary

noOfBorrowers is not being incremented inside BorrowLib when borrower redeposit.

Root Cause

Whenever borrower deposit for the first time there is an increment of noOfBorrowers in omniChainData global variable. The condition for increment is depositResult.borrowerIndex == 1

        if (depositResult.borrowerIndex == 1) {
            ++omniChainData.noOfBorrowers;
        }

Blockchian/contracts/lib/BorrowLib.sol#L770

Later when borrower decides to withdraw, the only condition is that deposited amount is 0

            if (interfaces.treasury.getTotalDeposited(msg.sender) == depositDetail.depositedAmountInETH) {
                --omniChainData.noOfBorrowers;
            }

Blockchian/contracts/lib/BorrowLib.sol#L916

This means that whenever any user decided to redeposit noOfBorrowers will not be incremented, but when he decides to withdraw -noOfBorrowers will be decreased. E.x. there is total 5 user with 5 deposits - noOfBorrowers=5. Later 1 user decide to deposit/redosit 5 times -> noOfBorrowers becomes 0 and no borrower will be able to withdraw their deposit.

There are some other issues like calculateCumulativeRate does depends on noOfBorrowers being calculated properly in ommnichain as well in CDS.sol

Internal pre-conditions

none

External pre-conditions

none

Attack Path

user deposit-withdraw-deposit again-withdraw again-deposit again

Impact

1. Borrowers will not be able to withdraw their deposits, funds will be stuck
2. incorrect calculateCumulativeRate

PoC

        it.only("Should withdraw ETH (between 0.8 and 1)",async function(){
            const {BorrowingContractA,TokenA,globalVariablesA,usdtA,CDSContractA} = await loadFixture(deployer);
            const timeStamp = await time.latest();
            await usdtA.connect(user1).mint(user1.getAddress(),10000000000)
            await usdtA.connect(user1).approve(CDSContractA.getAddress(),10000000000);

            const options = Options.newOptions().addExecutorLzReceiveOption(400000, 0).toHex().toString()
            let nativeFee = 0
            ;[nativeFee] = await globalVariablesA.quote(1,0, options, false)
            await CDSContractA.connect(user1).deposit(10000000000,0,true,10000000000,100000, { value: nativeFee.toString()});

            const depositAmount = ethers.parseEther("1");

            await BorrowingContractA.connect(user1).depositTokens(
                100000,
                timeStamp,
                [2,
                110000,
                ethVolatility,1,
                depositAmount],
                {value: (depositAmount +  BigInt(nativeFee))})
            
            await BorrowingContractA.calculateCumulativeRate();
            await TokenA.mint(user1.getAddress(),80000000);
            await TokenA.connect(user1).approve(await BorrowingContractA.getAddress(),await TokenA.balanceOf(user1.getAddress()));
            const {odosData, odosSignData} = await calculateUpsideToSwap(
                (await treasuryA.getBorrowing(await user1.getAddress(), 1))[1],
                99900,
                1e18,
                ZeroAddress,
                await treasuryA.getAddress(),
                await BorrowingContractA.getAddress()
            )
            const result = await globalVariablesA.getOmniChainData();
            // console.log(result);

            await BorrowingContractA.connect(user1).withDraw(
                await user1.getAddress(), 
                1,
                odosData,
                odosSignData,
                99900,
                timeStamp,
                {value: nativeFee});
            const tx = await treasuryA.getBorrowing(
                    await user1.getAddress(), 1)
            await expect(tx[1].withdrawed).to.be.equal(true);
            const result2 = await globalVariablesA.getOmniChainData();

            await BorrowingContractA.connect(user1).depositTokens(
                100000,
                timeStamp,
                [2,
                    110000,
                    ethVolatility,1,
                    depositAmount],
                {value: (depositAmount +  BigInt(nativeFee))})

            // expect(await treasuryA.noOfBorrowers()).to.be.equal(0) //0 noOfBorrowers
            expect(result2[17]).to.be.equal(0) //0 noOfBorrowers
            // console.log(result2);
        })

Mitigation

Move it before deposit to treasury

+        if (interfaces.treasury.getTotalDeposited(msg.sender) == 0) {
+            ++omniChainData.noOfBorrowers;
+        }
        //Call the deposit function in Treasury contract
        ITreasury.DepositResult memory depositResult = interfaces.treasury.deposit{
            value: params.assetName == IBorrowing.AssetName.ETH ? params.depositingAmount : 0
        }(
            msg.sender,