Auctioneer#auction stores routing data incorrectly which leads to loss of funds for sellers and bidders

Summary

The Auctioneer#auction function accepts routing data and stores it in the wrong storage slot, overwrites past data and results in loss of funds for all participants.

Vulnerability Detail

The Auctioneer#auction function is for starting a new auction, and accepts parameters from a seller. The routing parameters are fetched and then stored in the contract, in the variable lotRouting[lotId], where lotId is the new lot or auction's Id. This is iterated in the contract in Auctioneer#L194 (Link). However, the storage pointer to the variable is initialized much earlier in Auctioneer#L174 (Link).

This means that the routing parameters, including seller details and other inputted parameters are stored in lotRouting[0] at all times.

Impact

This results in a new auction always wiping out the routing data of the previous auction, and mismatch between the auction details stored in modules and the one stored in the Auctioneer/AuctionHouse contract. Therefore, funds are lost from all participants including the seller. Funds from older auctions get locked in the contract and can never be recovered.

Code Snippet

The affected code snippets are as follows :

The PoC to prove this vulnerability is as follows :

Please note that you will have to import console from the forge standard library to get console logs.


    function test_RoutingIncorectStorageSet()
        external
        whenAuctionTypeIsBatch
        whenBatchAuctionModuleIsInstalled
        givenQuoteTokenHasDecimals(13)
        givenBaseTokenHasDecimals(17)
    {
        //We will use two addresses - seller 1 and seller 2
        address seller1 = address(0x40);
        address seller2 = address(0x45);
        uint96 scaledAmount = _scaleBaseTokenAmount(_LOT_CAPACITY);
        _baseToken.mint(seller1, scaledAmount);
        _baseToken.mint(seller2, scaledAmount);

        vm.prank(seller1);
        _baseToken.approve(
            address(_auctionHouse),
            scaledAmount
        );

        vm.prank(seller2);
        _baseToken.approve(
            address(_auctionHouse),
            scaledAmount
        );

        vm.startPrank(seller1);
        uint96 _lotId1 = _auctionHouse.auction(
            _routingParams,
            _auctionParams,
            _INFO_HASH
        );
        vm.stopPrank();

        vm.prank(seller2);
        uint96 _lotId2 = _auctionHouse.auction(
            _routingParams,
            _auctionParams,
            _INFO_HASH
        );

        Auctioneer.Routing memory storageRouting1 = _getLotRouting(_lotId1);
        Auctioneer.Routing memory storageRouting2 = _getLotRouting(_lotId2);

        console.log("Expected seller 1 : ", seller1);
        console.log("Seller 1 in stored routing : ", storageRouting1.seller);
        console.log("Expected seller 2 : ", seller2);
        console.log("Seller 2 in stored routing : ", storageRouting2.seller);
    }

To test this, you can run forge test --mt test_RoutingIncorectStorageSet -vv

Tool used

Manual Review

Recommendation

Iterate the lotCounter before the storage variable is initialized.

Duplicate of #12

sherlock-audit / 2024-03-axis-finance-judging

devblixt - Auctioneer#auction stores routing data incorrectly which leads to loss of funds for sellers and bidders #151