Using endtime for order cancelation deadline of gnosis auction could lead to bait and switch bid tactics

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/plugins/trading/GnosisTrade.sol#L84-L157

Vulnerability details

Impact

Allowing users to cancel orders last moment can lead to bait and switch bids designed to obtain assets below market value

Proof of Concept

GnosisTrade.sol#L84-L157

function init(
    IBroker broker_,
    address origin_,
    IGnosis gnosis_,
    uint48 batchAuctionLength,
    TradeRequest calldata req
) external stateTransition(TradeStatus.NOT_STARTED, TradeStatus.OPEN) {

    ...

    broker = broker_;
    origin = origin_;
    gnosis = gnosis_;
    endTime = uint48(block.timestamp) + batchAuctionLength;

    ...

    auctionId = gnosis.initiateAuction(
        sell,
        buy,
        endTime, <- @audit orderCancellationEndDate
        endTime, <- @audit auctionEndDate
        _sellAmount,
        minBuyAmount,
        minBuyAmtPerOrder,
        0,
        false,
        address(0),
        new bytes(0)
    );
}

Above we see that when creating an auction, both the auction end and order cancellation end are set to the same value.

EasyAuction.sol#L152-L227

function initiateAuction(
    IERC20 _auctioningToken,
    IERC20 _biddingToken,
    uint256 orderCancellationEndDate,
    uint256 auctionEndDate,
    uint96 _auctionedSellAmount,
    uint96 _minBuyAmount,
    uint256 minimumBiddingAmountPerOrder,
    uint256 minFundingThreshold,
    bool isAtomicClosureAllowed,
    address accessManagerContract,
    bytes memory accessManagerContractData
) public returns (uint256) {

    ...

    auctionData[auctionCounter] = AuctionData(
        _auctioningToken,
        _biddingToken,
        orderCancellationEndDate,
        auctionEndDate,
        IterableOrderedOrderSet.encodeOrder(
            userId,
            _minBuyAmount,
            _auctionedSellAmount
        ),
        minimumBiddingAmountPerOrder,
        0,
        IterableOrderedOrderSet.QUEUE_START,
        bytes32(0),
        0,
        false,
        isAtomicClosureAllowed,
        feeNumerator,
        minFundingThreshold
    );

    ...
}

We see from the gnosis auction contract that these unmodified time values are used directly for the auction. The result is that orders can be canceled up the the block before it is finalized. This opens up the ability to carry out a bait and switch MEV tactic on the auction.

1. Bid entire auction amount with price 10% higher than market value at start of auction
2. Other parties see that all tokens are "sold" for a price much higher than they are willing to pay. Since bidding requires locking tokens, rational actors won't place lower bids
3. The block before it settles, cancel the high order and replace it with a bid at the minimum price

This would work particularly well as gnosis auctions can't be utilized by searchers/bots so a majority of bidders would be assumed to be humans.

Tools Used

Manual review

Recommended Mitigation Steps

orderCancellationEndDate should be set to a few minutes before the end of the auction, allowing for others to see the bait and switch and bid accordingly.

Assessed type

MEV

[akshatmittal]

This is a known issue and has been discussed before.

We actually want the people to be able to cancel their order anytime they want. The protocol already sets a minimum bound of what is acceptable in terms of price, and market participants come into the auction with the associated risks.

The protocol is not at risk here, it can get a price closer to the min clearing price, but that's acceptable for the protocol.

[thereksfour]

wadren raised an attack that would lead to value leaks, which meets the M severity. (Btw, I didn't find this issue from the report, if it was known I would invalidate it.

[c4-judge]

thereksfour marked the issue as satisfactory

[c4-judge]

thereksfour marked the issue as selected for report

[3docSec]

This is an interesting finding; for an impact to happen, we are assuming that:

• someone will actually make an above-market bid
• honest participants will see this bid
• honest participants will ignore that all bids can be canceled at the last second (despite it being a publicly visible setting)
• honest participants who do not like locking their tokens will not fall back to the most reasonable behavior, which is bidding close to the end of the auction so their tokens are in the worst case locked only shortly

These are very strong assumptions on the behaviour of users, but there does not seem to be an actual issue with the code itself. It is worth questioning whether running a batch auction without a cancellation buffer can be considered a vulnerability in itself, or it would be a hard requirement of the EasyAuction contract.

[akshatmittal]

Had a chat with @thereksfour about this. (reiterating here)

He mentioned: "I think it's M because it prevents the auction from being fulfilled at a better price, and while we think the minimum price is acceptable, it can lead to fulfillment at a low price even if a better price is available." which I believe is a good characterization based on the report.

However, that's already a property of this auction given that users need to lock and claim assets at the end of the auction. Additionally, we disable atomicSettlement to reinforce. By allowing users to cancel at the end, it allows them to participate in the auctions more freely and with greater intent. The protocol (Governance) already limits on how low you can go, effectively setting the lower bound on where users can participate.

[thereksfour]

Sorry for my previous misunderstanding, the highest bid does not block lower bids, so I will invalid it.

[c4-judge]

thereksfour marked the issue as unsatisfactory: Invalid