dimah7 - A malicious user can DoS the matching of offers

[sherlock-admin2]

dimah7

High

A malicious user can DoS the matching of offers

Summary

When users create a borrow order through the DebitaBorrowOffer-Factory, there is a orders counter variable which is tracking the number of total orders:

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/ce50bab1067574ae493f4062665b8e28611f2346/Debita-V3-Contracts/contracts/DebitaBorrowOffer-Factory.sol#L141

function createBorrowOrder(
        ...
        borrowOrderIndex[address(borrowOffer)] = activeOrdersCount;
        allActiveBorrowOrders[activeOrdersCount] = address(borrowOffer);
@>      activeOrdersCount++;
        ...

And when a order is accepted or canceled, as expected the variable is decremented:

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/ce50bab1067574ae493f4062665b8e28611f2346/Debita-V3-Contracts/contracts/DebitaBorrowOffer-Implementation.sol#L179
https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/ce50bab1067574ae493f4062665b8e28611f2346/Debita-V3-Contracts/contracts/DebitaBorrowOffer-Implementation.sol#L216

DebitaBorrowOffer-Implementation:

function acceptBorrowOffer(uint amount) public onlyAggregator nonReentrant onlyAfterTimeOut {
        ...
            IDBOFactory(factoryContract).emitDelete(address(this));
@>          IDBOFactory(factoryContract).deleteBorrowOrder(address(this));
        ...
    }

function cancelOffer() public onlyOwner nonReentrant {
        ...
@>      IDBOFactory(factoryContract).deleteBorrowOrder(address(this));
        IDBOFactory(factoryContract).emitDelete(address(this));
    }

As we can see the factory's deleteBorrowOrder() is called, where activeOrdersCount is decremented:

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/ce50bab1067574ae493f4062665b8e28611f2346/Debita-V3-Contracts/contracts/DebitaBorrowOffer-Factory.sol#L176

function deleteBorrowOrder(address _borrowOrder) external onlyBorrowOrder {
        ...
@>      activeOrdersCount--;
    }

However this can cause DoS issue for other lenders and borrowers

Root Cause

activeOrdersCount decrements both when an order is accepted and canceled.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Since any user can call DebitaV3Aggregator::matchOffersV3 that means a user is given the freedom to specify the parameters he wants, consider the following scenario:

• Bob creates a borrower offer activeOrdersCount will increment to 1
• A malicious user creates both a borrow and lend offer with small amounts as an asking amount and lending amount and low ratio parameters
• So activeOrdersCount will be 2
• Since there is no explicit check, requiring the total lent amount to equal the borrow order's requested amount, that means partial matching is allowed. So here the lent amount will be less than the asking amount (consider it 100 asking and 90 lending amount)
• The malicious user calls matchOffersV3, the function calculates the collateral amount and calls: DBOImplementation(borrowOrder).acceptBorrowOffer:

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/ce50bab1067574ae493f4062665b8e28611f2346/Debita-V3-Contracts/contracts/DebitaV3Aggregator.sol#L467-L483

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/ce50bab1067574ae493f4062665b8e28611f2346/Debita-V3-Contracts/contracts/DebitaV3Aggregator.sol#L573-L575

            ...
@>          uint userUsedCollateral = (lendAmountPerOrder[i] * (10 ** decimalsCollateral)) / ratio;

            // get updated weight average from the last weight average
            uint updatedLastWeightAverage = (weightedAverageRatio[principleIndex] * m_amountCollateralPerPrinciple) /
                (m_amountCollateralPerPrinciple + userUsedCollateral);

            // same with apr
            uint updatedLastApr = (weightedAverageAPR[principleIndex] * amountPerPrinciple[principleIndex]) /
                (amountPerPrinciple[principleIndex] + lendAmountPerOrder[i]);

            // add the amounts to the total amounts
            amountPerPrinciple[principleIndex] += lendAmountPerOrder[i];
@>          amountOfCollateral += userUsedCollateral;
            amountCollateralPerPrinciple[principleIndex] += userUsedCollateral;
            ...
@>          DBOImplementation(borrowOrder).acceptBorrowOffer(borrowInfo.isNFT ? 1 : amountOfCollateral);

• Above the amountOfCollateral will be subtracted from the available amount (so there will be some value left in the offer) and transfered to the aggregator contract:

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/ce50bab1067574ae493f4062665b8e28611f2346/Debita-V3-Contracts/contracts/DebitaBorrowOffer-Implementation.sol#L137-L179

function acceptBorrowOffer(
        uint amount
    ) public onlyAggregator nonReentrant onlyAfterTimeOut {
        BorrowInfo memory m_borrowInformation = getBorrowInfo();
        require(
            amount <= m_borrowInformation.availableAmount,
            "Amount exceeds available amount"
        );
        require(amount > 0, "Amount must be greater than 0");

@>      borrowInformation.availableAmount -= amount;

        // transfer collateral to aggregator
        if (m_borrowInformation.isNFT) {
            IERC721(m_borrowInformation.collateral).transferFrom(
                address(this),
                aggregatorContract,
                m_borrowInformation.receiptID
            );
        } else {
@>           SafeERC20.safeTransfer(
                IERC20(m_borrowInformation.collateral),
                aggregatorContract,
                amount
            );
        }
        ...
@>          IDBOFactory(factoryContract).deleteBorrowOrder(address(this));
        } else {
            IDBOFactory(factoryContract).emitUpdate(address(this));
        }
    }

• Then deleteBorrowOrder will be called which will decrement the activeOrdersCount to 1
• Since there some value left in the borrow order contract the malicious user (as owner) can call cancelOffer(), where he will get the leftover amount and deleteBorrowOrder will be invoked again, which means activeOrdersCount will be 0:

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/ce50bab1067574ae493f4062665b8e28611f2346/Debita-V3-Contracts/contracts/DebitaBorrowOffer-Implementation.sol#L216

function cancelOffer() public onlyOwner nonReentrant {
       ...
        require(availableAmount > 0, "No available amount");
        // set available amount to 0
        // set isActive to false
        borrowInformation.availableAmount = 0;
        ...
        } else {
            SafeERC20.safeTransfer(
                IERC20(m_borrowInformation.collateral),
                msg.sender,
                availableAmount
            );
        }
        ...
@>      IDBOFactory(factoryContract).deleteBorrowOrder(address(this));
        IDBOFactory(factoryContract).emitDelete(address(this));
    }

• Now when a Bob's order is tried to be matched the function will revert because it will try to decrement the 0-ed activeOrdersCount, which will DoS the matchOffersV3

Impact

This attack will DoS the borrowers and the lenders also since their offers can't be matched also

However this attack applies for early borrowers, it will be partially mitigated when there are a lot of borrow orders, but let's say that the protocol updates and all the borrowers now must cancel their offers to get their collateral back. However the last borrowers will not be able to get his funds back, because in normal scenario activeOrdersCount should be 1 for the last borrower, but instead will be 0 due to double decrement of the attack, hence it will lead to loss of funds for the last borrower

PoC

No response

Mitigation

No response