002.md

Happy Rouge Coyote

Medium

Index conflict in _deleteAuctionOrder: Deleting Auction Order Ambiguity

Summary

The _deleteAuctionOrder and every delete functions of Factory Contracts are designed to remove an active implementation, identified by its address, from the allActiveOrders array and update the corresponding index mappings. The function is intended to keep track of active orders by re-indexing them in a way that maintains order integrity while reducing the activeOrdersCount.

Root Cause

Here is an example of AuctionFactory Contract which uses the same logic for evey factory contract of protocol

The first creator of auction will get the 0th index of the AuctionOrderIndex mapping:

    function createAuction(
        uint _veNFTID,
        address _veNFTAddress,
        address liquidationToken,
        uint _initAmount,
        uint _floorAmount,
        uint _duration
    ) public returns (address) {
       ...
        // LOGIC INDEX
        AuctionOrderIndex[address(_createdAuction)] = activeOrdersCount;
       ...
    }

activeOrdersCount initially is set to 0 at declaration. Then at the _deleteAuctionOrder function sets the auction address to 0th index assuming that 0 is not used as valid index.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

1. This conflict can lead to incorrect behavior when other functions try to retrieve the index of auction orders, as they may mistakenly interpret a 0 index as either an active order at index 0 or a deleted order. This can lead to data inconsistencies.

2. Other functions that rely on AuctionOrderIndex to access or manage active orders may misinterpret a deleted order as an active one, causing unwanted errors or undefined behavior in the contract.

PoC

    function testCreateAuction() public {
        vm.startPrank(seller);

        veNFTContract.approve(address(factory), 1);
        auction = factory.createAuction(1, address(veNFTContract), address(AERO), 100, 50, 100);

        vm.stopPrank();
        uint index = factory.AuctionOrderIndex(auction);

        console.log("index of first auction %s", index);
        assertTrue(factory.isAuction(auction));

        address secondSeller = makeAddr("secondSeller");
        veNFTContract.mint(secondSeller, 2);
        vm.startPrank(secondSeller);

        veNFTContract.approve(address(factory), 2);
        address secondAuction = factory.createAuction(2, address(veNFTContract), address(AERO), 100, 50, 100);

        vm.stopPrank();
        
        uint index2 = factory.AuctionOrderIndex(secondAuction);
        console.log("index of second auction %s", index2);
        vm.prank(secondSeller);
        DutchAuction_veNFT(secondAuction).cancelAuction();
        
        index2 = factory.AuctionOrderIndex(secondAuction);
        console.log("index of second auction after cancel %s", index2);
    }

Logs:
  index of first auction 0
  index of second auction 1
  index of second auction after cancel 0

No response

Mitigation

Increment activeOrdersCount at the start of createAuction function to skip the 0 and reserve it for non-exisiting auctions.