Lively Goldenrod Pelican - Vulnerability in AuctionFactory.deleteAuction Function

[sherlock-admin2]

Lively Goldenrod Pelican

High

Vulnerability in AuctionFactory.deleteAuction Function

Summary

The deleteAuction function in the AuctionFactory contract contains a vulnerability that allows an auction owner to repeatedly call the function. This exploit manipulates the contract's state, ultimately resulting in the deletion of valid auctions. This could lead to unintended or malicious removal of auction orders, potentially impacting auction integrity and user trust.

This vulnerabilty is exploited based on the fact that _deleteAunctionOrder function can be repeatedly called even after a particular aunction has been deleted. during delete the aunction order is set to index zero.
Now a malicious actor can recall the function and cause a valid aunction sitting at index zero to be deleted, this can go on and on until all valid aunctions are removed

https://github.com/sherlock-audit/2024-11-debita-finance-v3/blob/main/Debita-V3-Contracts/contracts/auctions/Auction.sol#L168-L184

function _deleteAuctionOrder(address _AuctionOrder) external onlyAuctions {
        // get index of the Auction order
        uint index = AuctionOrderIndex[_AuctionOrder];//iterates index
        AuctionOrderIndex[_AuctionOrder] = 0;//sets order to zero

        // get last Auction order
        allActiveAuctionOrders[index] = allActiveAuctionOrders[
            activeOrdersCount - 1//sets last aunction to removed index
        ];
        // take out last Auction order
        allActiveAuctionOrders[activeOrdersCount - 1] = address(0);

        // switch index of the last Auction order to the deleted Auction order
        AuctionOrderIndex[allActiveAuctionOrders[index]] = index;
        activeOrdersCount--;
    }

Root Cause

Root cause is in the Aunction.sol#cancelAunction which doesn;t check if an aunction has been previously deleted

 function cancelAuction() public onlyActiveAuction onlyOwner {
        s_CurrentAuction.isActive = false;  //no checks to ensure that an inactive aunction can't be deleted again
        // Send NFT back to owner
        IERC721 Token = IERC721(s_CurrentAuction.nftAddress);
        Token.safeTransferFrom(
            address(this),
            s_ownerOfAuction,
            s_CurrentAuction.nftCollateralID
        );

        auctionFactory(factory)._deleteAuctionOrder(address(this));
        auctionFactory(factory).emitAuctionDeleted(
            address(this),
            s_ownerOfAuction
        );
        // event offerCanceled
    }

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

An attacker identifies that the deleteAuctionOrder function lacks proper validation and safeguards. and calls the function repeatedly until all valid aunctions are removed

Impact

Malicious actors could exploit this vulnerability to delete valid auctions, potentially causing financial loss to auction participants and damaging trust in the platform.

PoC

No response

Mitigation

add require statement to Aunction.cancelAunction to dissallow already cancelled aunctions

 function cancelAuction() public onlyActiveAuction onlyOwner {
 require{s_CurrentAuction.isActive, "aunction already cancelled"}
        s_CurrentAuction.isActive = false;
        // Send NFT back to owner
        IERC721 Token = IERC721(s_CurrentAuction.nftAddress);
        Token.safeTransferFrom(
            address(this),
            s_ownerOfAuction,
            s_CurrentAuction.nftCollateralID
        );

        auctionFactory(factory)._deleteAuctionOrder(address(this));
        auctionFactory(factory).emitAuctionDeleted(
            address(this),
            s_ownerOfAuction
        );
        // event offerCanceled
    }